spin.c includes pg_sema.h even if unnecessary
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:
dont_include_pg_sema_when_HAVE_SPINLOCK.patchtext/x-patch; charset=us-asciiDownload+4-2
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
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 doesIDK, 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