[PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing

[Giuseppe Bilotta]

On Tue, Feb 6, 2018 at 1:01 AM, Keith Packard <keithp at keithp.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta at gmail.com> writes:
>
>> I'm not a big fan of strtod because with it it's impossible to know if
>> a conversion actually happened. xrandr --scale '  ' would actually be
>> accepted (resulting in a scale value of 0), while the scanf catches
>> it.
>
> strtod takes an 'endptr' argument which can be used for precisely this
> purpose,

Not in the sense I mean above. If the string is a sequences of
whitespace characters, strtod would return 0 as value (no conversion),
and set endptr to the end of the string, because it would gobble the
whitespace. This would be indistinguishable from it successfully
parsing a string argument of, say, '0.0'.

> but I think your use of sscanf is easier to read as it does
> both conversions in one call, and lets you pick the two different
> versions easily (--scale 2 and --scale 2x1.5). One could imagine doing
>
>         xscale = strtod(string, &endptr);
>         if (*endptr) {
>                 if (endptr == string || *endptr != 'x')
>                         syntax error;
>                 string = endptr + 1;
>                 yscale = strtod(string, &endptr);
>                 if (endptr == string || *endptr)
>                         syntax error;
>         } else {
>                 yscale = xscale;
>
> That's a lot more code than two calls to sscanf...

I interpreted Walter's suggestion as using strtod only in if the
sscanf failed to find 2 values, since then we expect a single value.

>> Of course there's also to be said that we could reject a scale factor
>> of 0, regardless of whether it comes from a correct parsing of the
>> string '0.0' or from the parse of an empty string (but of course then
>> we couldn't customize the error message to differentiate between
>> “incorrect parse” and “value out of range”).
>
> Probably a good thing to catch.

True, we should do this regardless of how the values are parsed. Also
ensure that it's not negative.

Hm. Since we have to do it both for scale and gamma, I wonder if it's
overengineering if I try to refactor this parsing of "N or 1 positive
values with separator S" into its own function.

-- 
Giuseppe "Oblomov" Bilotta