[PATCH] Remove unnecessary non-standard & unportable inclusions.
Jan Beulich
jbeulich@suse.com
Mon Nov 4 12:51:57 GMT 2024
On 04.11.2024 12:53, Jonas 'Sortie' Termansen wrote:
> Thanks for the quick review Jan :)
>
> On 11/4/24 09:01, Jan Beulich wrote:
>> The question with such #include-s is virtually always: Are they there to
>> work around issues on certain platforms (a detail which would imo be
>> relevant to put in the patch description)? In the case here the line was
>> present from the introduction of the helper tool, so that's hard to
>> answer (may want to ask the author though, as it hasn't been _that_ long
>> ago that the tool was introduced). On that basis I'd be okay to approve
>> that change alone, while ...
>
> Good question and honestly hard to test with the very long tail of systems
> that have ported binutils. (I really need to rebuild my lab with VMs of
> every single operating system I can get my hands on.)
>
> But even then: Such systems would fail to implement C89 with <string.h>
> which is a requirement to build binutils. The file in question is new and
> looks to be extremely plain C besides unistd.h to get chdir. I doubt it
> has any surprising portability requirements.
>
> Binutils has a HAVE_MEMORY_H macro, but it's not used anywhere (besides
> an indirect use through the bundled readline), making it very likely such
> workarounds are not needed for anything, since it would probably be needed
> in more places in binutils if it was.
>
> I'd say it's safe to remove. If not, we're dealing with a platform that
> doesn't define memset in <string.h> and everything else would break.
Right, hence why I said "On that basis I'd be okay to approve that change
alone". Implying that you would go and split the patch.
>>> <sys/param.h> is not needed and not standardized and contains a kitchen
>>> sink of various unportable definitions not agreed upon and best done
>>> manually or through other headers.
>>
>> ... I guess we will want Nick's input here, whom you didn't even Cc.
>
> Who's Nick? Oh the author of the file.
More precisely (for the context here) the maintainer of libctf, who you
should Cc when sending patches.
Jan
More information about the Binutils
mailing list