Re: [PATCH] windres: add quotes around preprocessor cmd if needed

["Clément Chigot" <chigot@adacore.com>]

On Tue, Jun 28, 2022 at 3:52 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.06.2022 14:26, Clément Chigot wrote:
> > On Tue, Jun 28, 2022 at 11:37 AM Clément Chigot <chigot@adacore.com> wrote:
> >>
> >> On Tue, Jun 28, 2022 at 11:00 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 28.06.2022 09:35, Clément Chigot via Binutils wrote:
> >>> Shouldn't you also (optionally) quote the pre-processor string
> >>> if that came into read_rc_file() as non-NULL? Everything else looks
> >>> okay to me.
> >>
> >> Yeah. I didn't run into this issue but looking at the code it should happen
> >> too. Thanks for pointing it out !
> >
> > Actually the space issue is already handled by windres.c:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=binutils/windres.c;h=4df821ee8451bda70490d6626707c2bdd66eeb5f;hb=HEAD#l887
> > It's not as complete as it should be. But apart of "%" I'm not sure
> > any other characters caught by "filename_need_quotes" can happen
> > in a folder name under Windows.
>
> Oh, I see. I'm not sure I see a strong need for dealing with %, so
> I'm wondering: Are you intending to make an updated patch, or do
> you want to commit the one you've got (which I'm okay with given
> your observation)?

Honestly, if this patch fits you, I would merge it as is. This issue is already
a corner case, but as it's hidden inside windres code and because of
the existence of the "Program Files" folder can be problematic.
The other has to be triggered by someone explicitly passing "--preprocessor"
with an unusual path. So honestly I don't think it would ever happen.

> > Making "filename_need_quotes" more global might be a good idea
> > anyway.
>
> Right, but that could be a separate change then.

Agreed.