Created attachment 14210 [details] [review]
Add some argument parsing checks
xrandr's argument parsing is done very loosely with a lot of scanf()s and atoi()s. As a result, trailing garbage is accepted on almost any numeric argument, and sometimes altogether garbage is accepted as a numeric argument of 0. The first attached patch fixes much of this, but more work would be needed to fix it all.
More importantly, the name given to --output is not checked in any way. If a nonexistent output is specified, xrandr silently does nothing. The second attached patch fixes this, so that a warning is issued after execution if one or more of the specified outputs was not found.
Created attachment 14211 [details] [review]
Give a warning at the end for each --output that was not found on the server
probably too late for 7.6, unless you want to merge it and do a new stable release, matthias? if so, please go ahead.
Some changes remove a reasonable warning, e.g.
- screen = atoi (argv[i]);
- if (screen < 0) usage();
+ screen = check_strtol(argv[i]);
It would be worthwhile to verify whether check_strtol() could validate for positive or 0 values always.
Also, I'd love to have the two fixes (double vs. float and check_strto*()) in separate commits. But that is no showstopper.
Bart, can you fix the patch accordingly?
I like the second patch, though I think 'mark' is a misnomer, 'found' is probably better. I'll apply it.
David, what hast to be done to apply this to 7.6? A release, yes, what else?
Created attachment 29220 [details] [review]
revised version of previous patch
(In reply to comment #3)
> Some changes remove a reasonable warning, e.g.
> - screen = atoi (argv[i]);
> - if (screen < 0) usage();
> + screen = check_strtol(argv[i]);
Good point. Fixed.
> Also, I'd love to have the two fixes (double vs. float and check_strto*()) in
> separate commits. But that is no showstopper.
IIRC (and it's been a while) the only reason to move the floats to doubles was to avoid writing check_strtof(). So I just changed the commit message. :-)
> Bart, can you fix the patch accordingly?
Any more patches to be applied? Otherwise I'll do another release.
No, I'm good for now. Thanks for dealing with it.
At some point someone should replace the whole argument parsing mechanism for this program with something better. However, that's a big job and I'm afraid I don't really feel like taking it on right this second.