https://sourceware.org/bugzilla/show_bug.cgi?id=27594
--- Comment #11 from Martin Storsjö <martin at martin dot st> --- (In reply to Eli Zaretskii from comment #10) > > 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'. Yes, that's true: We need to make sure windres receives a pre-quoted string, thus the caller has to double quote it. > > 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? Indeed, I think that was a mistake in the revert. I'll try to follow up with a patch for reverting the right thing in a couple hours, and for clarifying the documentation regarding it - to codify the existing status quo regarding it. > > 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? Yeah the change in quot doesn't affect --preprocessor indeed. The "you" in this section was more of a generic towards the project, not you (singular) in particular. > > 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. Yes I agree that the most sane thing to do would be to have the user specify -DSTRING="quotedstring", but there are projects that have ended up working around this, so fixing this case does break them. I'll follow up with a separate bug for that topic in a couple hours, with more examples and details, to avoid derailing this one further. -- You are receiving this mail because: You are on the CC list for the bug.
