Thread: s_lock.h busted
The weekend's hacking on s_lock.h broke it for all platforms that need non-default definitions of S_UNLOCK or S_INIT_LOCK (hpux, alpha, a couple others). Someone put unconditional definitions of those macros at the bottom of the file. I suspect this was a plain old editing typo, but perhaps the intent was to put such definitions in one of the platform-specific #if blocks? (If so, they were unnecessary anyway.) Anyhow, the attached patch fixes it for hpux. regards, tom lane *** src/include/storage/s_lock.h.orig Mon Jul 20 12:05:59 1998 --- src/include/storage/s_lock.h Mon Jul 20 13:04:49 1998 *************** *** 323,332 **** #define TAS(lock) tas((volatile slock_t *) lock) #endif /* TAS */ - #define S_UNLOCK(lock) (*(lock) = 0) - - #define S_INIT_LOCK(lock) S_UNLOCK(lock) - #endif /* HAS_TEST_AND_SET */ #endif /* S_LOCK_H */ --- 323,328 ----
> The weekend's hacking on s_lock.h broke it for all platforms that > need non-default definitions of S_UNLOCK or S_INIT_LOCK (hpux, > alpha, a couple others). Someone put unconditional definitions > of those macros at the bottom of the file. I suspect this was a > plain old editing typo, but perhaps the intent was to put such > definitions in one of the platform-specific #if blocks? (If so, > they were unnecessary anyway.) Anyhow, the attached patch fixes > it for hpux. > It came in from: Somewhere between 6.1 and 6.3 someone removed the support for the NS32K machine I contributed. In any case, I now have postgresql-6.3 running again on NetBSD/pc532, a NS32532 machine. The following changes are needed relative to the src directory. (It looks like support was partially removed when the files were moved from the src/backend/storage/.... tree to the src/include tree.) If you need me to get a current development version of postgresql for this change let me know. Also, let me know if this code needs updating due to another code movement that deleted the old NS32K support. Thank you. Phil Nelson Fix applied. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> > The weekend's hacking on s_lock.h broke it for all platforms that > need non-default definitions of S_UNLOCK or S_INIT_LOCK (hpux, > alpha, a couple others). Someone put unconditional definitions > of those macros at the bottom of the file. I suspect this was a > plain old editing typo, but perhaps the intent was to put such > definitions in one of the platform-specific #if blocks? (If so, > they were unnecessary anyway.) Anyhow, the attached patch fixes > it for hpux. > > regards, tom lane > > > *** src/include/storage/s_lock.h.orig Mon Jul 20 12:05:59 1998 > --- src/include/storage/s_lock.h Mon Jul 20 13:04:49 1998 > *************** > *** 323,332 **** > #define TAS(lock) tas((volatile slock_t *) lock) > #endif /* TAS */ > > - #define S_UNLOCK(lock) (*(lock) = 0) > - > - #define S_INIT_LOCK(lock) S_UNLOCK(lock) > - > > #endif /* HAS_TEST_AND_SET */ > #endif /* S_LOCK_H */ > --- 323,328 ---- > Arrrrgggghhh!!!! Ok, I'm calmer now... These were meant to be in the conditional blocks at the end of the file so that if (and only if) no definition existed we would get a default. So: #ifndef S_UNLOCK #define S_UNLOCK(lock) (*(lock) = 0) #endif #ifndef S_INIT_LOCK #define S_INIT_LOCK(lock) S_UNLOCK(lock) #endif I am a little concerned about the recent batch of patches made to this code. I was planning a cleanup patch to resolve all the issues raised, but kept seeing other patches and since I got badly burned by a merge conflict I was hoping it would settle down a little. Sigh... Perhaps I need to pull the latest tree again and see where we have gotten to. -dg David Gould dg@illustra.com 510.628.3783 or 510.305.9468 Informix Software (No, really) 300 Lakeside Drive Oakland, CA 94612 - If simplicity worked, the world would be overrun with insects. -
dg@illustra.com (David Gould) writes: > Arrrrgggghhh!!!! > These were meant to be in the conditional blocks at the end of the file so > that if (and only if) no definition existed we would get a default. So: > #ifndef S_UNLOCK > #define S_UNLOCK(lock) (*(lock) = 0) > #endif > #ifndef S_INIT_LOCK > #define S_INIT_LOCK(lock) S_UNLOCK(lock) > #endif Right, but those default definitions were *already there*. The lines I was complaining about were added immediately after the default definitions, and overrode *any* prior definition of the macros. As far as I can see they were just a typo/thinko. regards, tom lane