Re: [PATCH] sim, sim/m32c, sim/rl78: Use getopt_long

[Tsukasa OI <research_trasio@irq.a4lg.com>]

On 2022/10/24 16:51, Tsukasa OI wrote:
> On 2022/10/22 1:27, Tom Tromey wrote:
>>>>>>> Tsukasa OI via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> Because of Binutils/GCC hack, getopt on GNU libc (2.25 or before) is
>>> currently unusable on sim, causing a regression on CentOS 7.  Getting rid of
>>> include/getopt.h is the best solution to avoid hacking
>>
>> I don't think we want to do remove this file.  getopt_long is required
>> by a bunch of programs in the tree, and is part of libiberty -- but it's
>> not in the traditional getopt.h.
>>
>> Using getopt_long seems like an improvement though.
> 
> Glibc (2.25 or before; not too old to drop support) includes <getopt.h>
> from <unistd.h> but this causes <unistd.h> to include Binutils/GCC's
> include/getopt.h.  It means, using the filename "getopt.h" in an include
> path (-I) itself is a risk.  The only reason it didn't cause a problem
> is most of Binutils/GCC components used getopt_long and
> getopt_long_only, not plain getopt.
> 
> After I started checking getopt in sim/configure (to reduce build
> failure on Clang), it became a problem on sim because some sim files
> actually use plain getopt.
> 
> c.f. Following thread under binutils
> <https://sourceware.org/pipermail/binutils/2022-October/123543.html>
> 
>>
>>> but as a short-term
>>> solution, this commit replaces calls to getopt to call getopt_long.
>>
>> I notice you didn't update sim/ppc... why is that?
> 
> Because I simply missed that (I grepped "getopt (" and two occurrences
> under sim/ppc uses "getopt(" without space between "getopt" and "(" ).
> 
> Thanks for spotting that and I'll fix it soon.
> 
> Thanks,
> Tsukasa

There was another reason (I should have tested the branch before
submission and I wondered why I missed those getopt calls): for sim/ppc
files, it includes <getopt.h> before including project's config.h,
making getopt declarations not suppressed.

Replacing getopt with getopt_long will make this fact not relevant and
I'll just replace the function without modifying #include order (that
should be handled as a part of future "build with Clang" patchset).

Regards,
Tsukasa

> 
>> If those don't have the problem then perhaps we can just use that
>> solution, whatever it is.
>>
>> Tom
>>