Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi
On Thursday 18 October 2018 09:03:27 Giuseppe Bilotta wrote:
> Hello,
>
> On Thu, Apr 12, 2018 at 8:53 PM Pali Rohár 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.00 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.ro...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi
Hello,
On Thu, Apr 12, 2018 at 8:53 PM Pali Rohár 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.
> + 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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi
Gentle reminder for a patch which I sent 5 months ago...
On Monday 07 May 2018 23:38:10 Pali Rohár wrote:
> Hello, can you review my patch below?
>
> On Thursday 12 April 2018 20:52:21 Pali Rohár wrote:
> > Function strtod() sets strtod_error to the pointer of the first invalid
> > character and therefore it does not have to be first character from input.
> > When input is valid then it points to nul byte. Conversion error is
> > indicated by setted errno. Zero-length argument and zero DPI is invalid
> > too.
> >
> > Update also error message about invalid argument.
> >
> > Signed-off-by: Pali Rohár
> > ---
> > xrandr.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/xrandr.c b/xrandr.c
> > index 7f1e867..1960bbf 100644
> > --- a/xrandr.c
> > +++ b/xrandr.c
> > @@ -3115,9 +3115,10 @@ main (int argc, char **argv)
> > }
> > if (!strcmp ("--dpi", argv[i])) {
> > char *strtod_error;
> > - 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]);
> > + errno = 0;
> > dpi = strtod(argv[i], &strtod_error);
> > - if (argv[i] == strtod_error)
> > + if (*strtod_error || errno || dpi == 0)
> > {
> > dpi = 0.0;
> > dpi_output_name = argv[i];
> > @@ -3567,7 +3568,7 @@ main (int argc, char **argv)
> > XRROutputInfo *output_info;
> > XRRModeInfo *mode_info;
> > if (!dpi_output)
> > - fatal ("Cannot find output %s\n", dpi_output_name);
> > + fatal ("%s is not valid DPI nor valid output\n",
> > dpi_output_name);
> > output_info = dpi_output->output_info;
> > mode_info = dpi_output->mode_info;
> > if (output_info && mode_info && output_info->mm_height)
>
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi
Hello, can you review my patch below?
On Thursday 12 April 2018 20:52:21 Pali Rohár wrote:
> Function strtod() sets strtod_error to the pointer of the first invalid
> character and therefore it does not have to be first character from input.
> When input is valid then it points to nul byte. Conversion error is
> indicated by setted errno. Zero-length argument and zero DPI is invalid
> too.
>
> Update also error message about invalid argument.
>
> Signed-off-by: Pali Rohár
> ---
> xrandr.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xrandr.c b/xrandr.c
> index 7f1e867..1960bbf 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -3115,9 +3115,10 @@ main (int argc, char **argv)
> }
> if (!strcmp ("--dpi", argv[i])) {
> char *strtod_error;
> - 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]);
> + errno = 0;
> dpi = strtod(argv[i], &strtod_error);
> - if (argv[i] == strtod_error)
> + if (*strtod_error || errno || dpi == 0)
> {
> dpi = 0.0;
> dpi_output_name = argv[i];
> @@ -3567,7 +3568,7 @@ main (int argc, char **argv)
> XRROutputInfo *output_info;
> XRRModeInfo *mode_info;
> if (!dpi_output)
> - fatal ("Cannot find output %s\n", dpi_output_name);
> + fatal ("%s is not valid DPI nor valid output\n",
> dpi_output_name);
> output_info = dpi_output->output_info;
> mode_info = dpi_output->mode_info;
> if (output_info && mode_info && output_info->mm_height)
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH app/xrandr] Fix checking for valid argument of --dpi
Function strtod() sets strtod_error to the pointer of the first invalid
character and therefore it does not have to be first character from input.
When input is valid then it points to nul byte. Conversion error is
indicated by setted errno. Zero-length argument and zero DPI is invalid
too.
Update also error message about invalid argument.
Signed-off-by: Pali Rohár
---
xrandr.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/xrandr.c b/xrandr.c
index 7f1e867..1960bbf 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -3115,9 +3115,10 @@ main (int argc, char **argv)
}
if (!strcmp ("--dpi", argv[i])) {
char *strtod_error;
- 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]);
+ errno = 0;
dpi = strtod(argv[i], &strtod_error);
- if (argv[i] == strtod_error)
+ if (*strtod_error || errno || dpi == 0)
{
dpi = 0.0;
dpi_output_name = argv[i];
@@ -3567,7 +3568,7 @@ main (int argc, char **argv)
XRROutputInfo *output_info;
XRRModeInfo *mode_info;
if (!dpi_output)
- fatal ("Cannot find output %s\n", dpi_output_name);
+ fatal ("%s is not valid DPI nor valid output\n",
dpi_output_name);
output_info = dpi_output->output_info;
mode_info = dpi_output->mode_info;
if (output_info && mode_info && output_info->mm_height)
--
2.11.0
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
