Re: *PING* [PATCH] libiberty: allow comments in option file
On 10/5/2021 10:15 AM, Hans-Peter Nilsson wrote: On Sat, 25 Sep 2021, Hu Jialun wrote: Hello, Sorry for bumping it again but I guess it was getting overlooked. I am very junior with mailing list open source contributions so please feel free to point out if I have inadvertantly done something in an incorrect way. The archive of the original email can be found at https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579232.html and is also appended below. Best regards, Hu Jialun This looks useful, I hope a reviewer will eventually notice it. Until then keep pinging (say, weekly)! I read the upstream thread from a while back and I got the impression the utility was in question, so I pushed this near the bottom of my review list. jeff
Re: *PING* [PATCH] libiberty: allow comments in option file
On Sat, 25 Sep 2021, Hu Jialun wrote: > Hello, > > Sorry for bumping it again but I guess it was getting overlooked. > > I am very junior with mailing list open source contributions so please feel > free to point out if I have inadvertantly done something in an incorrect way. > > The archive of the original email can be found at > https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579232.html > and is also appended below. > > Best regards, > Hu Jialun This looks useful, I hope a reviewer will eventually notice it. Until then keep pinging (say, weekly)! A few comments (no pun intended): > > > Enables putting line comments into option files included using '@'. > > > > Comments begin with a ';' followed by whitespace, and terminates at the > > end of the line. Backward compability should be satisfactory since ';' > > is a character disallowed in DOS filenames and rarely used in filenames > > or options anywhere else as well. Perhaps mention that the ';' has to be at the start of the line to be considered a comment? Also, while ';' is, as you say, very suitable considering the context, maybe allow the ubiquitous '#' as well? Though, I don't know how common that is in MSWindows and Apple-anything file-paths, in corner cases. Thoughts? > > > > Related discussion: https://gcc.gnu.org/legacy-ml/gcc/2011-02/msg00422.html > > --- > > libiberty/argv.c | 5 + > > libiberty/testsuite/test-expandargv.c | 12 > > 2 files changed, 17 insertions(+) > > > > diff --git a/libiberty/argv.c b/libiberty/argv.c > > index 48dcd102461..2bc7569b718 100644 > > --- a/libiberty/argv.c > > +++ b/libiberty/argv.c > > @@ -194,6 +194,11 @@ char **buildargv (const char *input) > > { > > /* Pick off argv[argc] */ > > consume_whitespace (&input); I'd suggest a comment here as to the purpose, e.g. /* Skip comments. */ > > + if (*input == ';') > > + { > > + for (; *input != '\n' && *input != EOS; ++input); The ";" (for the empty block) should be at a line by itself, says the coding-standard. > > + continue; > > + } > > > > if ((maxargc == 0) || (argc >= (maxargc - 1))) > > { > > diff --git a/libiberty/testsuite/test-expandargv.c > > b/libiberty/testsuite/test-expandargv.c > > index 56c170f9ec6..5640b2b41cf 100644 > > --- a/libiberty/testsuite/test-expandargv.c > > +++ b/libiberty/testsuite/test-expandargv.c > > @@ -142,6 +142,18 @@ const char *test_data[] = { > >"b", > >0, > > > > + /* Test 7 - Check for comments in option file. */ > > + "abc\n;def\nxy \\;z \\ ;gh",/* Test 7 data */ > > + ARGV0, > > + "@test-expandargv-7.lst", > > + 0, > > + ARGV0, > > + "abc", > > + "xy", > > + ";z", > > + " ;gh", > > + 0, > > + > >0 /* Test done marker, don't remove. */ > > }; > > > > -- > > 2.33.0 > Thanks and good luck! brgds, H-P
*PING* [PATCH] libiberty: allow comments in option file
Hello, Sorry for bumping it again but I guess it was getting overlooked. I am very junior with mailing list open source contributions so please feel free to point out if I have inadvertantly done something in an incorrect way. The archive of the original email can be found at https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579232.html and is also appended below. Best regards, Hu Jialun > Enables putting line comments into option files included using '@'. > > Comments begin with a ';' followed by whitespace, and terminates at the > end of the line. Backward compability should be satisfactory since ';' > is a character disallowed in DOS filenames and rarely used in filenames > or options anywhere else as well. > > Related discussion: https://gcc.gnu.org/legacy-ml/gcc/2011-02/msg00422.html > --- > libiberty/argv.c | 5 + > libiberty/testsuite/test-expandargv.c | 12 > 2 files changed, 17 insertions(+) > > diff --git a/libiberty/argv.c b/libiberty/argv.c > index 48dcd102461..2bc7569b718 100644 > --- a/libiberty/argv.c > +++ b/libiberty/argv.c > @@ -194,6 +194,11 @@ char **buildargv (const char *input) > { > /* Pick off argv[argc] */ > consume_whitespace (&input); > + if (*input == ';') > + { > + for (; *input != '\n' && *input != EOS; ++input); > + continue; > + } > > if ((maxargc == 0) || (argc >= (maxargc - 1))) > { > diff --git a/libiberty/testsuite/test-expandargv.c > b/libiberty/testsuite/test-expandargv.c > index 56c170f9ec6..5640b2b41cf 100644 > --- a/libiberty/testsuite/test-expandargv.c > +++ b/libiberty/testsuite/test-expandargv.c > @@ -142,6 +142,18 @@ const char *test_data[] = { >"b", >0, > > + /* Test 7 - Check for comments in option file. */ > + "abc\n;def\nxy \\;z \\ ;gh",/* Test 7 data */ > + ARGV0, > + "@test-expandargv-7.lst", > + 0, > + ARGV0, > + "abc", > + "xy", > + ";z", > + " ;gh", > + 0, > + >0 /* Test done marker, don't remove. */ > }; > > -- > 2.33.0