Re: [PATCH] include: Declare getopt function on old GNU libc

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

On 2022/10/17 20:52, Pedro Alves wrote:
> On 2022-10-16 2:12 p.m., Tsukasa OI wrote:
>> On 2022/10/13 20:59, Pedro Alves wrote:
>>> On 2022-10-13 11:11 a.m., Tsukasa OI via Binutils wrote:
>>>> On GNU libc <= 2.25, <unistd.h> includes <getopt.h> with __need_getopt macro
>>>> defined.  That <getopt.h> is intended to be a part of GNU libc but
>>>> <unistd.h> actually includes include/getopt.h in this project.
>>>
>>> Messy.
>>
>> Well, I have to agree.
>>
>>>
>>> Do we really still need this getopt.h header?
>>>
>>> gnulib, at:
>>>
>>>  https://www.gnu.org/software/gnulib/manual/html_node/getopt_002eh.html
>>>
>>> says:
>>>
>>>   "This header file is missing on some platforms: AIX 5.1, HP-UX 11, MSVC 14."
>>>
>>> AIX 5.1 is from 2001.
>>> HP-UX 11 is from 1997.
>>> We don't support building with MSVC AFAIK.
>>>
>>> Can't we just get rid of it?
>>>
>>
>> I feel this is too unsafe to do that
> 
> Why?  Are you aware of any host system that people actually build binutils on
> that doesn't have a proper getopt declaration?

No.  I'm worrying because currently included getopt.h contains
declarations of getopt_long and getopt_long_only.  Those declarations
are required (not necessarily in <getopt.h> though) because if the
system does not have getopt_long and/or getopt_long_only, libiberty
implementation is (and should be) used.

>> (unless you assume getopt_long and
>> getopt_long_only are always available on the system).

That's why I said the sentence above.

> 
> getopt is supposed to be declared in unistd.h.  The .c files that
> use getopt (not the GNU extensions) could just switch to including
> that one.
> 
> Even if there is such a system, I would think that a better fix would
> be to rename include/getopt.h to something else, and have that header include
> <getopt.h> and/or do whatever else needed to pick the right declarations on the system.
> A standard header replacement that doesn't #include_next the original is just asking
> for trouble, like we've run into...

I also agree that.  In fact, that was my first idea to deal with this
problem.  As a long-term solution, I support your idea.

But, as a short-term solution, there are a few obstacles and intentions:

1.  The fact that include/getopt.h is shared by both Binutils and GCC
    projects so that we must sync Binutils and GCC.
    That would be far beyond my capacity to handle.  My messy patch also
    changed include/getopt.h but I thought I can explain the changes
    to GCC people (since the changes were that small).
2.  I don't want to disrupt any release schedule.  The changes will not
    be small and spans through multiple subprojects.
3.  For me, it's fine as long as...
    a.  CentOS 7 regression is resolved and
    b.  Build problem with Clang is fewer than before

Replacing getopt with getopt_long won't be large and relatively
self-explanatory.  I will propose sim workaround as a short-term solution.
_At the same time_, we could discuss the possibility of getting rid of
"include/getopt.h" entirely.

Thanks,
Tsukasa

> 
>>
>> Even if this patch is unacceptable, I don't want to revert previous
>> changes to sim/configure{,.ac} either (it's necessary to prevent a build
>> failure with Clang).  
> 
> It's only necessary because of this hacky include/getopt.h header existing, no?.
> Why would you want to keep the configure.ac bits if the hacky header with the
> unprototyped declaration doesn't exist any more?
> 
> Thanks,
> Pedro Alves
> 
>> It's also a hack but to stop using getopt
>> (entirely) may be an option, changing following files:
>>
>> -   sim/igen/igen.c
>> -   sim/m32c/main.c
>> -   sim/rl78/main.c
>>
>> I mean, we could replace getopt with getopt_long plus dummy longopts.
>> This way, CentOS (7) regression will (also) be gone.
>>
>> What do you think?
>>
>> Thanks,
>> Tsukasa
>>
>