On Sep 5 13:27, Mingye Wang wrote: > This commit rewrites the cmdline parser to achieve the following: > * MSVCRT compatibility. Except for the single-quote handling (an > extension for compatibility with old Cygwin), the parser now > interprets option boundaries exactly like MSVCR since 2008. This fixes > the issue where our escaping does not work with our own parsing. > * Clarity. Since globify() is no longer responsible for handling the > opening and closing of quotes, the code is much simpler. > * Sanity. The GLOB_NOCHECK flag is removed, so a failed glob correctly > returns the literal value. Without the change, anything path-like > would be garbled by globify's escaping. > * A memory leak in the @file expansion is removed by rewriting it to use > a stack of buffers. This also simplifies the code since we no longer > have to move stuff. The "downside" is that tokens can no longer cross > file boundaries. > > Some clarifications are made in the documentation for when globs are not > expanded. The functions are made public for testing, but my tcl setup > is currently too messed up for running them! I did test them as an > isolated program on WSL-Debian. > > The change fixes two complaints of mine: > * That cygwin is incompatible with its own escape.[1] > * That there is no way to echo `C:\"` from win32.[2] > [1]: https://cygwin.com/pipermail/cygwin/2020-June/245162.html > [2]: https://cygwin.com/pipermail/cygwin/2019-October/242790.html > > (It's never the point to spawn cygwin32 from cygwin64. Consistency > matters: with yourself always, and with the outside world when you are > supposed to.) > > This is the fourth version of the patch. I provide all my patches to > Cygwin, including this one and all future ones, under the 2-clause BSD > license.
Great, thanks. A couple of (mostly minor) nits, though. > diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din > index a7b4aa2b0..f15dc6a9e 100644 > --- a/winsup/cygwin/common.din > +++ b/winsup/cygwin/common.din > @@ -393,6 +393,8 @@ cygwin_split_path NOSIGFE > cygwin_stackdump SIGFE > cygwin_umount SIGFE > cygwin_winpid_to_pid SIGFE > +cygwin_cmdline_parse SIGFE > +cygwin_cmdline_build SIGFE Nope, we won't do that. The command line parsing is an internal thing, and we won't export arbitrary internal functions using their own symbol. *If* we should export this stuff at all, which I highly doubt as necessary, it should use the cygwin_internal API. > diff --git a/winsup/cygwin/include/sys/cygwin.h > b/winsup/cygwin/include/sys/cygwin.h > index 805671ef9..e19ac0cd2 100644 > --- a/winsup/cygwin/include/sys/cygwin.h > +++ b/winsup/cygwin/include/sys/cygwin.h > @@ -86,6 +86,8 @@ extern void *cygwin_create_path (cygwin_conv_path_t what, > const void *from); > extern pid_t cygwin_winpid_to_pid (int); > extern int cygwin_posix_path_list_p (const char *); > extern void cygwin_split_path (const char *, char *, char *); > +extern int cygwin_cmdline_parse (char *, char ***, char **, int, int); > +extern char *cygwin_cmdline_build (const char * const *, int, int); Ditto. > +static char* __reg1 > +read_file (const char *name) > +{ > +#ifndef WINF_STDIO_TEST Please drop this, together with the else branch for WSL. > + // For anything else, sort out backslashes first. > + // All backslashes are literal, except these before a quote. > + // Single-quote is our addition. Would love to remove it. Pleae use /* */ coments for multiline comments. > +/* Perform a glob on word if it contains wildcard characters. > + Also quote every character between quotes to force glob to > + treat the characters literally. > + > + Call glob(3) on the word, and fill argv accordingly. > + If the input looks like a DOS path, double up backslashes. > + */ Please join the last two lines. The closing */ should be on the last comment line. > +extern "C" int > +extern "C" char * > +extern "C" > +{ > + int cygwin_cmdline_parse (char *, char ***, char **, int, int); > + char *cygwin_cmdline_build (const char * const *, int, int); > +} Bzz. > --- a/winsup/doc/misc-funcs.xml > +++ b/winsup/doc/misc-funcs.xml Ditto. > diff --git a/winsup/testsuite/Makefile.in b/winsup/testsuite/Makefile.in > index a86a35b88..bdc116d12 100644 > --- a/winsup/testsuite/Makefile.in > +++ b/winsup/testsuite/Makefile.in Skip it. Just add this as a standalone testcase as attachement, that's sufficient. Thanks, Corinna