[PATCH xserver 1/3] xfree86: factor out the check priviliges and print a big warning

Emil Velikov emil.l.velikov at gmail.com
Mon May 2 22:39:38 UTC 2016


On 2 May 2016 at 14:45, Adam Jackson <ajax at nwnk.net> wrote:
> On Sun, 2016-05-01 at 18:34 +0100, Emil Velikov wrote:
>> Ping ?
>
> This faithfully preserves the original behavior, but...
>
>> > +static void
>> > +xf86CheckPrivs(const char *option, const char *arg, const char *path_file,
>> > +               const char *dfault)
>> > +{
>> > +    if (xf86PrivsElevated() && !xf86PathIsSafe(arg)) {
>> > +        FatalError("\nInvalid argument for %s\n"
>> > +                    "\tWith elevated privileges, the %s specified with %s must\n"
>> > +                    "\tinclude a relative path and must not contain any \"..\"\n"
>> > +                    "\telements.\n"
>> > +                    "\tUsing default %s %s.\n\n",
>> > +                    option, path_file, option, dfault, path_file);
>> > +    }
>> > +}
>
> This message is a lie, we do not in fact use the default, because
> FatalError() means we never go any further. Clearly nobody can be
> relying on that fallback working, so I'd just rip out the "default" bit
> and leave it as a fatal error.
>
> Also if I'm nitpicking, that error text is trash. Suggest "With
> elevated privileges, %s must specify a relative path without any \"..\"
> elements." % (option).
>
I barely managed to parse anything past the second line. With this
wording things read so much better.
I'm going to take your suggestions and improve a tiny bit - going to
print the actual invalid argument.

v2 coming in a second.

Thanks
Emil


More information about the xorg-devel mailing list