[Bug binutils/27594] build processes broken by changed space handling

[eliz at gnu dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27594

--- Comment #10 from Eli Zaretskii <eliz at gnu dot org> ---
(In reply to Martin Storsjö from comment #9)
> (In reply to Eli Zaretskii from comment #8)
> > Quoting with backslashes also doesn't work in cmd.exe.  The suggestion to
> > pre-quote the preprocessor is also unreliable, because when `windres` is run
> > from cmd.exe or from the native MinGW Make, the program's startup code
> > unquotes the argument, so the result will depend on how many "unquoting"
> > levels the command line is subject to.
> 
> I would beg to differ here; when any shell, be it either bash or cmd.exe,
> interprets a command line, it does split and unescape the command line
> depending on that shell's rules.
> 
> It's true that the actual splitting and unescaping happens at different
> levels (in the caller, in the case of unix shells, and in the callee in the
> case of a win32 executable), but the basic principle is still the same; if I
> want to call a tool and have it receive a specific argument, I have to quote
> that argument according to the current shell's rules (and those rules differ
> with respect to certain details).

Up until here, full agreement.

> So if I want to call windres so that it receives the argument "path
> to/gcc.exe" -E -DRC_INVOKED, I can call windres with "\"path to/gcc.exe\" -E
> -DRC_INVOKED" in cmd.exe and in bash - that particular quoting happens to
> work in both. In bash I could also call it with single quotes and not
> needing to escape the double quotes, as '"path to/gcc.exe" -E -DRC_INVOKED'.

Here is where I disagree: the "\"path to/gcc.exe\" or '"path to/gcc.exe"' is
NOT quoting, it's double quoting.  Quoting is just "path to/gcc.exe" or (for
Posix shells) 'path to/gcc.exe'.

> The quot() function isn't called for --preprocessor
> arguments, it's only called for --preprocessor-arg and -D and similar. It
> still just wraps the whole --preprocessor argument in double quotes, here:
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=binutils/windres.c;
> h=2b626a29fb68ada1cc012a267a60d72c1335bea9;
> hb=23182ac0d832477d316547ec2a758d22b43d0837#l889

Yes, and thus I don't understand why the change to `quot` was reverted.  If
that function's job is to quote something, then it should quote correctly on
MS-Windows, and the only sensible quoting on MS-Windows is "like this".  If
someone wants to argue that arguments with whitespace should not be quoted,
then why do we still call `quot` on Unix?

> But the core issue - that the --preprocessor argument used to accept
> multiple arguments (and where spaces would have to be pre-quoted) - do you
> want to willingly break that, which multiple existing projects rely on? 

You just said that the change in `quot` doesn't affect --preprocessor, so how
can the change in `quot` break that (incorrect) usage of --preprocessor?  Why
was it reverted?  What am I missing here?

> Eli - separately, the new quoting behaviour (that was reverted now,
> needlessly?) is, in principle a good thing, but it's still missing to quote
> a few tricky cases - namely backslashes and double quotes. And fixing that
> would be breaking roughly a similar number of projects.

The code does attempt to quote double quotes, but if that code is buggy, please
show the specific use case where it breaks.  If the code is incomplete, let's
fix that, by all means.

As for backslashes: they don't need to be quoted on native Windows, only if you
are using a Posix shell, in which case the user should have typed the original
file name or string argument with backslashes already doubled, right?

> If I want to preprocess with the option e.g. -DSTRING="quotedstring", I want
> windres to call popen with -DSTRING=\"quotedstring\", so I need to call
> windres with -DSTRING=\\\"quotedstring\\\".

I don't think I agree.  The user should say -DSTRING="quotedstring" and the
code which invokes `popen` should then take care of quoting any arguments that
have embedded whitespace.  Otherwise, we will have to know exactly how many
"unquoting" levels will the string undergo, and add quotes for each level. 
That way lies madness, because it is not practical to do that -- you never know
how many such levels will the string undergo.  The only sane way is for the
code which is going to pass a string to a shell command to quote it if needed,
and as appropriate to the command to be invoked.

> So it would in theory be great
> if quot() would handle proper escaping of backslashes and double quotes, but
> if we'd do that change, we'd be breaking those existing users that have
> codified that case of double escaping for such strings.

I still want to see those existing users which we will break.  I suspect that
either they are already broken (and use loopholes due to bugs in the
implementation), or will simply be unaffected.

So by all means, let's talk about those use cases one by one, and see what we
want to do about them.

-- 
You are receiving this mail because:
You are on the CC list for the bug.