Обсуждение: 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