[v2] sim, sim/{m32c,ppc,rl78}: Use getopt_long
Commit Message
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(-)
base-commit: a5a0a4fd0ff536a8dbd532864cfc095a306b678c
Comments
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
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
On 26 Oct 2022 19:57, Tsukasa OI wrote:
> 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.
i would make it static const too in that case so it isn't constructed on
the stack ... it's just part of the rodata and loaded right away.
> Ah, a minor question to you, Mike. Can I consider your "lgtm" as an
> approval for specific area you are responsible?
"lgtm" is equiv to "approved, please push when you're ready"
-mike
On 2022/10/27 1:01, Mike Frysinger wrote:
> On 26 Oct 2022 19:57, Tsukasa OI wrote:
>> 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.
>
> i would make it static const too in that case so it isn't constructed on
> the stack ... it's just part of the rodata and loaded right away.
That makes sense. I submitted PATCH v4:
<https://sourceware.org/pipermail/gdb-patches/2022-October/193200.html>
and I will push this version in this weekend unless there's an objection
from someone.
>
>> Ah, a minor question to you, Mike. Can I consider your "lgtm" as an
>> approval for specific area you are responsible?
>
> "lgtm" is equiv to "approved, please push when you're ready"
> -mike
Thanks! I will push some of my "build with Clang" work with plain
"lgtm" response in this weekend.
Tsukasa
@@ -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 };
lf *standard_out =
lf_open ("-", "stdout", lf_omit_references, lf_is_text, "igen");
@@ -1162,8 +1163,9 @@ main (int argc, char **argv, char **envp)
printf (" -t <output-file> output itable\n");
}
- while ((ch = getopt (argc, argv,
- "B:D:F:G:H:I:M:N:P:T:W:o:k:i:n:hc:d:e:m:r:s:t:f:x"))
+ while ((ch = getopt_long (argc, argv,
+ "B:D:F:G:H:I:M:N:P:T:W:o:k:i:n:hc:d:e:m:r:s:t:f:x",
+ &dummy_longopts, NULL))
!= -1)
{
#if 0 /* For debugging. */
@@ -29,6 +29,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include <setjmp.h>
#include <signal.h>
#include <sys/types.h>
+#include <getopt.h>
#ifdef HAVE_SYS_SOCKET_H
#ifdef HAVE_NETINET_IN_H
@@ -135,12 +136,14 @@ main (int argc, char **argv)
#ifdef HAVE_networking
char *console_port_s = 0;
#endif
+ struct option dummy_longopts = { 0 };
setbuf (stdout, 0);
in_gdb = 0;
- while ((o = getopt (argc, argv, "tc:vdm:C")) != -1)
+ while ((o = getopt_long (argc, argv, "tc:vdm:C", &dummy_longopts, NULL))
+ != -1)
switch (o)
{
case 't':
@@ -271,6 +271,7 @@ main(int argc,
{
lf_file_references file_references = lf_include_references;
spreg_table *sprs = NULL;
+ struct option dummy_longopts = { 0 };
char *real_file_name = NULL;
int is_header = 0;
int ch;
@@ -284,8 +285,9 @@ main(int argc,
printf("-L Suppress cpp line numbering in output files\n");
}
-
- while ((ch = getopt(argc, argv, "hLsn:r:p:")) != -1) {
+ while ((ch = getopt_long (argc, argv, "hLsn:r:p:", &dummy_longopts, NULL))
+ != -1)
+ {
#if 0 /* For debugging. */
fprintf(stderr, "\t-%c %s\n", ch, ( optarg ? optarg : ""));
#endif
@@ -351,6 +351,7 @@ main(int argc,
filter *filters = NULL;
insn_table *instructions = NULL;
table_include *includes = NULL;
+ struct option dummy_longopts = { 0 };
char *real_file_name = NULL;
int is_header = 0;
int ch;
@@ -390,9 +391,11 @@ main(int argc,
printf(" -f <output-file> output support functions\n");
}
- while ((ch = getopt(argc, argv,
- "F:EI:RSLJT:CB:H:N:o:k:i:n:hc:d:m:s:t:f:"))
- != -1) {
+ while (
+ (ch = getopt_long (argc, argv, "F:EI:RSLJT:CB:H:N:o:k:i:n:hc:d:m:s:t:f:",
+ &dummy_longopts, NULL))
+ != -1)
+ {
#if 0 /* For debugging. */
fprintf(stderr, "\t-%c %s\n", ch, (optarg ? optarg : ""));
#endif
@@ -64,10 +64,12 @@ main (int argc, char **argv)
int save_trace;
bfd *prog;
int rc;
+ struct option dummy_longopts = { 0 };
xmalloc_set_program_name (argv[0]);
- while ((o = getopt (argc, argv, "tvdr:D:M:")) != -1)
+ while ((o = getopt_long (argc, argv, "tvdr:D:M:", &dummy_longopts, NULL))
+ != -1)
{
switch (o)
{