Re: [PATCH v2] sim, sim/{m32c,ppc,rl78}: Use getopt_long

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

On 2022/10/26 17:59, Mike Frysinger wrote:
> On 25 Oct 2022 06:27, Tsukasa OI wrote:
>> Because of Binutils/GCC hack, getopt on GNU libc (2.25 or earlier) is
>> currently unusable on sim, causing a regression on CentOS 7.
>>
>> This is caused as follows:
>>
>> 1.  If HAVE_DECL_GETOPT is defined (getopt with known prototype is
>>     declared), a declaration of getopt in "include/getopt.h" is suppressed.
>>     The author started to define HAVE_DECL_GETOPT in sim with the commit
>>     340aa4f6872c ("sim: Check known getopt definition existence").
>> 2.  GNU libc (2.25 or earlier)'s <unistd.h> includes <getopt.h> to declare
>>     getopt function (only, not getopt_long or getopt_long_only) but it
>>     causes <unistd.h> to include Binutils/GCC's "include/getopt.h".
>> 3.  If both 1. and 2. are satisfied, despite that <unistd.h> tries to
>>     declare getopt by including <getopt.h>, "include/getopt.h" does not
>>     define one, causing getopt function unusable.
>>
>> Getting rid of "include/getopt.h" (e.g. renaming this header file) is the
>> best solution to avoid hacking but as a short-term solution, this commit
>> replaces getopt with getopt_long under sim/.
>> ---
>>  sim/igen/igen.c | 6 ++++--
>>  sim/m32c/main.c | 5 ++++-
>>  sim/ppc/dgen.c  | 6 ++++--
>>  sim/ppc/igen.c  | 9 ++++++---
>>  sim/rl78/main.c | 4 +++-
>>  5 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/sim/igen/igen.c b/sim/igen/igen.c
>> index ba856401fa9..22cfd30ec43 100644
>> --- a/sim/igen/igen.c
>> +++ b/sim/igen/igen.c
>> @@ -989,6 +989,7 @@ main (int argc, char **argv, char **envp)
>>    char *real_file_name = NULL;
>>    int is_header = 0;
>>    int ch;
>> +  struct option dummy_longopts = { 0 };
> 
> just call it "longopts" so we don't have to rename it in the future if we
> decide to actually add long options.  comes up in the other files too.
> 
> otherwise lgtm.
> -mike

To prepare actual long options, not just renaming, making them array of
struct option seems better.  Moving longopts out from the caller is
avoided for now (since it might not get big so that it requires option
definition outside a function).  That means...

Before: struct option dummy_longopts = { 0 };
After:  struct option longopts[] = { { 0 } };

I fixed like this and I'll submit PATCH v3 (with fix above and minor
commit message clarification) soon.  Finally, I can clean up the mess I
created.

Ah, a minor question to you, Mike.  Can I consider your "lgtm" as an
approval for specific area you are responsible?

Best regards,
Tsukasa