Обсуждение: Re: pgsql: Fix headerscheck failure in replication/worker_internal.h
On 2021-Nov-16, Alvaro Herrera wrote:
> Fix headerscheck failure in replication/worker_internal.h
The other failure is in src/include/libpq/be-gssapi-common.h:
In file included from /tmp/headerscheck.a6f0um/test.c:2:
./src/include/libpq/be-gssapi-common.h:20:10: fatal error: gssapi/gssapi.h: No existe el fichero o el directorio
20 | #include <gssapi/gssapi.h>
| ^~~~~~~~~~~~~~~~~
compilation terminated.
One possible solution for this one is to add an exclusion in
headerscheck (and cpluspluscheck?); the other possible solution seems to
be to wrap the whole contents of the file in "#ifdef ENABLE_GSS". I
think the latter is roughly the approach used for OpenSSL inclusions.
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
Greetings, * Alvaro Herrera (alvherre@alvh.no-ip.org) wrote: > On 2021-Nov-16, Alvaro Herrera wrote: > > > Fix headerscheck failure in replication/worker_internal.h > > The other failure is in src/include/libpq/be-gssapi-common.h: > > In file included from /tmp/headerscheck.a6f0um/test.c:2: > ./src/include/libpq/be-gssapi-common.h:20:10: fatal error: gssapi/gssapi.h: No existe el fichero o el directorio > 20 | #include <gssapi/gssapi.h> > | ^~~~~~~~~~~~~~~~~ > compilation terminated. > > One possible solution for this one is to add an exclusion in > headerscheck (and cpluspluscheck?); the other possible solution seems to > be to wrap the whole contents of the file in "#ifdef ENABLE_GSS". I > think the latter is roughly the approach used for OpenSSL inclusions. Short answer is yes, inclusion of be-gssapi-common.h is typically wrapped in a #ifdef, see src/backend/libpq/auth.c Thanks, Stephen
Вложения
On 2021-Nov-16, Stephen Frost wrote: > Short answer is yes, inclusion of be-gssapi-common.h is typically > wrapped in a #ifdef, see src/backend/libpq/auth.c It'd be as in the attached, then. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Вложения
Greetings,
On Tue, Nov 16, 2021 at 12:33 Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2021-Nov-16, Stephen Frost wrote:
> Short answer is yes, inclusion of be-gssapi-common.h is typically
> wrapped in a #ifdef, see src/backend/libpq/auth.c
It'd be as in the attached, then.
I’ve not looked at this very closely but no, normally it’s the inclusion of be-gssapi-common.h that’s wrapped in the ifdef, not the contents of it, again, see auth.c
Not against possibly changing that but I don’t get the point of including be-gssapi-common.h if it’s not enabled in the build and typically if GSSAPI is possible and the reason for including be-gssapi-common.h then there’s other things that need to be under a ifdef, again, as in auth.c
If there isn’t any need to be-gssapi-common.h then maybe just drop that include instead of adding an ifdef..?
Thanks,
Stephen
On 2021-Nov-16, Stephen Frost wrote: > I’ve not looked at this very closely but no, normally it’s the inclusion of > be-gssapi-common.h that’s wrapped in the ifdef, not the contents of it, > again, see auth.c I don't think you read my original post very carefully. I'm talking about sanitizing the output of the headerscheck script. -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/ "Java is clearly an example of money oriented programming" (A. Stepanov)
On 2021-Nov-16, Stephen Frost wrote: > Not against possibly changing that but I don’t get the point of including > be-gssapi-common.h if it’s not enabled in the build and typically if GSSAPI > is possible and the reason for including be-gssapi-common.h then there’s > other things that need to be under a ifdef, again, as in auth.c BTW, this is exactly why my first suggestion was to add an exclusion rule to headerscheck so that be-gssapi-common.h is not verified by that script. After re-reading your response, that looks like a reasonable answer too. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
Greetings,
On Tue, Nov 16, 2021 at 13:16 Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2021-Nov-16, Stephen Frost wrote:
> Not against possibly changing that but I don’t get the point of including
> be-gssapi-common.h if it’s not enabled in the build and typically if GSSAPI
> is possible and the reason for including be-gssapi-common.h then there’s
> other things that need to be under a ifdef, again, as in auth.c
BTW, this is exactly why my first suggestion was to add an exclusion
rule to headerscheck so that be-gssapi-common.h is not verified by that
script. After re-reading your response, that looks like a reasonable
answer too.
Yeah, that seems better to me, though I’ve not yet had time to look deeply into any of this.
Thanks,
Stephen
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Nov-16, Stephen Frost wrote:
>> Not against possibly changing that but I don’t get the point of including
>> be-gssapi-common.h if it’s not enabled in the build and typically if GSSAPI
>> is possible and the reason for including be-gssapi-common.h then there’s
>> other things that need to be under a ifdef, again, as in auth.c
> BTW, this is exactly why my first suggestion was to add an exclusion
> rule to headerscheck so that be-gssapi-common.h is not verified by that
> script. After re-reading your response, that looks like a reasonable
> answer too.
I think adding #ifdef ENABLE_GSS as per your prior message is better.
Headers have little business making assumptions about the context in
which they're included --- which is exactly why headerscheck exists ---
so I disagree with Stephen's argument. In any case I am not in favor of
making random exclusions from that script's testing.
regards, tom lane
Greetings,
On Tue, Nov 16, 2021 at 15:12 Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Nov-16, Stephen Frost wrote:
>> Not against possibly changing that but I don’t get the point of including
>> be-gssapi-common.h if it’s not enabled in the build and typically if GSSAPI
>> is possible and the reason for including be-gssapi-common.h then there’s
>> other things that need to be under a ifdef, again, as in auth.c
> BTW, this is exactly why my first suggestion was to add an exclusion
> rule to headerscheck so that be-gssapi-common.h is not verified by that
> script. After re-reading your response, that looks like a reasonable
> answer too.
I think adding #ifdef ENABLE_GSS as per your prior message is better.
Headers have little business making assumptions about the context in
which they're included --- which is exactly why headerscheck exists ---
so I disagree with Stephen's argument. In any case I am not in favor of
making random exclusions from that script's testing.
I don’t feel all that strongly either way, so if you’d rather have it that way then that’s fine. Will still need the other ifdefs too anyway though, but I guess it isn’t that big of a deal.
Thanks,
Stephen