Re: [PATCH] Cygwin: rewrite cmdline parser

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

Hi Mingye,

On Nov  7 20:12, 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 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 looks already pretty good now, but there's a problem when building:

  winsup/cygwin/winf.cc:67:1: error: no declaration matches ‘bool linebuf::fromargv(av&, const char*, bool, bool)’
     67 | linebuf::fromargv (av& newargv, const char *real_path, bool trunc_for_cygwin, bool forcequote)
	| ^~~~~~~
  In file included from winsup/cygwin/winf.cc:16:
  winsup/cygwin/winf.h:76:15: note: candidate is: ‘bool linebuf::fromargv(av&, const char*, bool)’
     76 |   bool __reg3 fromargv(av&, const char *, bool);;
	|               ^~~~~~~~
  winsup/cygwin/winf.h:64:7: note: ‘class linebuf’ defined here
     64 | class linebuf
	|       ^~~~~~~

The declaration in winf.h is actually missing the "forcequote" arg.  Is
"forcequote" supposed to be a default parameter?  If so, defaulting to
true or false? 

However, given that *both* calls to fromargv() don't set forcequote at
all, it will be the same value for all callers and thus it's entirely
redundant.  So why not just drop it?

Also, this change

> +#if defined (__x86_64__) || defined (__CYGMAGIC__) || !defined (__GNUC__)

is entirely unrelated and should go into it's own patch explaining
why it's necessary.  After all, we're building Cygwin with gcc only...


Thanks,
Corinna