[PATCH app/xrandr] Fix checking for valid argument of --dpi

Pali Rohár pali.rohar at gmail.com
Thu Oct 18 07:36:45 UTC 2018


On Thursday 18 October 2018 09:03:27 Giuseppe Bilotta wrote:
> Hello,
> 
> On Thu, Apr 12, 2018 at 8:53 PM Pali Rohár <pali.rohar at gmail.com> wrote:
> > -           if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> > +           if (++i >= argc || !argv[i][0]) argerr ("%s requires an argument\n", argv[i-1]);
> 
> I don't think this change is necessary, if arg[i][0] is NULL it means
> there _was_ an argument, but it was empty. Getting different error
> messages from `xrandr --dpi ''` and `xrandr --dpi ' '` doesn't seem
> like a good idea to me.

strtod() does not signal error for empty string argument:

  char *endptr = NULL;
  double ret;
  errno = 0;
  ret = strtod("", &endptr);
  printf("ret=%lf errno=%d end=%x\n", ret, errno, endptr[0]);

  ret=0.000000 errno=0 end=0

But with explicit check for zero or negative return value, it is not
really needed.

> > +           errno = 0;
> >             dpi = strtod(argv[i], &strtod_error);
> > -           if (argv[i] == strtod_error)
> > +           if (*strtod_error || errno || dpi == 0)
> 
> While we're at it, I would make the check for dpi <= 0, since negative
> values aren't valid either (in fact, negative values are effectively a
> no-op, since they set the DPI from the current framebuffer settings,
> and then set the virtual framebuffer physical dimensions from the
> DPI).
> 
> Cheers,
> 
> Giuseppe Bilotta

-- 
Pali Rohár
pali.rohar at gmail.com


More information about the xorg-devel mailing list