[PATCH] Remove unnecessary non-standard & unportable inclusions.

[Jonas 'Sortie' Termansen]

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.

>> <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. Hi! I'm Jonas, this is my first
binutils contributions, nice to meet you. I didn't know we were supposed
to cc the original authors :)

The file uses roundup() which is in <sys/param.h> but it also defines its
own fallback. Looking further into it, glibc actually has a better
definition if __GNUC__ that optimizes constant powers of two.

I'd say the options here are to either just remove the inclusions and rely
on the fallback, or to just wrap it in HAVE_SYS_PARAM_H as a better solution
that gets the improved glibc definitions.

What do you think?

Jonas