Am Freitag, 11. Juli 2025, 18:42:34 Mitteleuropäische Sommerzeit schrieb Jason
Merrill:
> On 7/10/25 4:41 PM, Nicolas Werner wrote:
> > Users might be using a space in their build directory path. To allow
> > specifying such a root for the module mapper started by GCC, we need the
> > command to allow quotes. Previously quoting a path passed to the module
> > mapper was not possible, so replace the custom argv parsing with
> > the argv parsing logic from libiberty, that supports fairly standard
> > shell quoting using single and double quotes.
> >
> > This also should fix PR110153, although my intention was really to fix
> > passing parameters to the "--root" parameter.
> >
> > I don't know how to best add a test with this yet, since I am unsure
> > about how to best deal with spaces in test folders.
>
> Can you be more specific?
I don't know how to reliably pass a space through the test files and on IRC
people mostly agreed, that leaving out a test for this could be an option.
But more specifically, I am trying to test invocations like
-fmodule-mapper="@g++-module-mapper --root 'path with space'"
or
-fmodule-mapper="@g++-module-mapper --root 'path with space'"
I don't know what quoting rules apply to dg-additional-options and I also
don't know if spaces in file names are even supported on all platforms the
tests need to run on.
>
> > - if (!arg_no)
> > - {
> > - /* @name means look in the compiler's install dir. */
> > - if (ptr[0] == '@')
> > - ptr++;
> > - else
> > - full_program_name = nullptr;
> > - }
> > -
> > - argv[arg_no++] = ptr;
> > - while (*ptr && *ptr != ' ')
> > - ptr++;
> > - if (!*ptr)
> > - break;
> > - *ptr = 0;
> > - }
> > + // Split mapper argument into parameters
> > + char** original_argv = buildargv (name.c_str () + 1);
> > + int arg_no = countargv (original_argv);
> > + char **argv = new char *[arg_no + 1];
>
> Can we drop original_argv so argv is the result of buildargv without
> copying?
If we do that, then I think freeargv() won't work properly, since it will call
free() on a char* incremented by 1. I couldn't think if a simpler way to avoid
that. Decrementing that argument again doesn't seem like a good solution.
Swapping the arg 0 with a different pointer might work.
>
> > + for (int i = 0; i < arg_no; i++)
> > + argv[i] = original_argv[i];
> > +
> > + if (arg_no && argv[0][0] == '@')
> > + argv[0] = argv[0] + 1;
>
> Let's keep the comment from the old code.
Oops, yes, will do!
>
> > @@ -108,8 +89,8 @@ spawn_mapper_program (char const **errmsg, std::string
> > &name,
>
> Word wrap in your mail client corrupted the patch here; it may be easier
> to attach it to avoid that.
>
> Jason
Ah, my bad, I will fix that. I only checked if it didn't wrap any +- lines, I
didn't notice this one. I'll just turn the wrapping off next time.
Thanks for the feedback!
Nico
