Re: [PATCH] libctf,libsframe: remove include of config.h

On 14 Aug 2026, Indu Bhagat told this:
> On 2026-08-11 09:18, Andrew Burgess wrote:
>> However, in most cases we would expect the main source file to already
>> pull in config.h, so it feels easier to just make that the requirement
>> and remove the include of config.h from libctf/swap.h.
>> Removing the include of config.h seemed risky so I looked at all the
>> files currently pulling in libctf/swap.h, there are not many:
>> libctf/ctf-endian.h:
>>    - Includes config.h before including swap.h
>> libctf/ctf-open-bfd.c:
>> libctf/ctf-open.c:
>>    - Both include ctf-impl.h, which includes config.h before including
>>      swap.h
>> libsframe/sframe.c:
>>    - This is our problem case, pulls in its own local config.h before
>>      including libctf/swap.h.
>> So I think making this change should be safe, and resolves the
>> warnings when performing an in tree build.

My worry here is simple: libctf/swap.h is a direct user of HAVE_*
symbols from libctf's swap.h. Remove it, and suddenly any future user
which doesn't remember to include libctf's config.h will silently
misbehave. However, as a countervailing matter, since ctf-impl.h
includes config.h, and it is universally included by libctf source
files, this is probably harmless and I'm worrying about nothing.

It's just... ugly.

> While current usages of swap.h in libctf/ will do fine with this
> change, I am concerned for posterity because if any future user of
> swap.h does not include ctf-impl.h or config.h, it may lead to
> undesirable outcomes (unintended usage of swap.h definitions when
> those from byteswap.h exist, etc ?).

Exactly.

> (For the short term) A possibility is to duplicate this header in
> libsframe and at least keep the two directories not cross-use. I will
> send a patch shortly for this adding a local copy (say,
> libsframe/sframe-swap.h) and removing -I$(srcdir)/../libctf from
> libsframe/Makefile.am. This will keep libsframe cleanly decoupled
> without altering libctf/swap.h.

I think this makes most sense, though the code duplication is a bit
unpleasant.

> Another option is to move the swap.h related functions to a common
> place like include/ in binutils so libsframe, libctf (and elfcpp?) can
> use it.

The problem there is that it also depends on configury changes, which
individual consumers would still have to remember.

-- 
NULL && (void)