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
