Thread: 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/

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/

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