spin.c includes pg_sema.h even if unnecessary

Started by Kyotaro Horiguchiover 8 years ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t38193
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 27, 2026 at 09:10 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t38193_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t38193_1 && git checkout t38193_1

Patchset v1 (message #1) is on t38193_1

Jump to latest
#1Kyotaro Horiguchi
horikyota.ntt@gmail.com

Ouch! Sorry for the bogus subject of just-submitted mail.
I'll make another thread with the fixed subject.

====
Hello.

As in another mail just before, spin.c seems a bit strange
(without acutual harm).

spin.h doesn't include pg_sema.h when HAVE_SPINLOCKS is defined,
but spin.c always includes it even in the case. The file is
included only to use sizeof(PGSemaphore) to calcualte
SpinlockSemaSize as 0.

The codes that requires PGSempaphore is inactivated when the
symbol is defined in all other places so it seems to me that we
ought to refrain from using it there, too. The attched patch does
that.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

t38193_1
dont_include_pg_sema_when_HAVE_SPINLOCK.patchtext/x-patch; charset=us-asciiDownload+4-2
#2Andres Freund
andres@anarazel.de
In reply to: Kyotaro Horiguchi (#1)
Re: spin.c includes pg_sema.h even if unnecessary

On 2018-02-15 20:11:07 +0900, Kyotaro HORIGUCHI wrote:

As in another mail just before, spin.c seems a bit strange
(without acutual harm).

spin.h doesn't include pg_sema.h when HAVE_SPINLOCKS is defined,
but spin.c always includes it even in the case. The file is
included only to use sizeof(PGSemaphore) to calcualte
SpinlockSemaSize as 0.

The codes that requires PGSempaphore is inactivated when the
symbol is defined in all other places so it seems to me that we
ought to refrain from using it there, too. The attched patch does

IDK, I don't quite see the point of the change here...

Greetings,

Andres Freund

#3Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Andres Freund (#2)
Re: spin.c includes pg_sema.h even if unnecessary

At Thu, 15 Feb 2018 10:01:57 -0800, Andres Freund <andres@anarazel.de> wrote in <20180215180157.7q55aytrhif7vvzw@alap3.anarazel.de>

On 2018-02-15 20:11:07 +0900, Kyotaro HORIGUCHI wrote:

As in another mail just before, spin.c seems a bit strange
(without acutual harm).

spin.h doesn't include pg_sema.h when HAVE_SPINLOCKS is defined,
but spin.c always includes it even in the case. The file is
included only to use sizeof(PGSemaphore) to calcualte
SpinlockSemaSize as 0.

The codes that requires PGSempaphore is inactivated when the
symbol is defined in all other places so it seems to me that we
ought to refrain from using it there, too. The attched patch does

IDK, I don't quite see the point of the change here...

No actual gain, but I just feel it uneasy that utterly-unused
symbol is used to yield zero. From other point of view, it seems
to be inconsistency that a header file is disabled in a file, but
not disabled in another on one configuration.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center