[BUG?] xrandr - Screen display died after setting brightness with wrong value

[cardosaum]

Hi, all!

Last weekend I had an unfortunate error, that I presume was a bug with xrandr.
when I executed this command

$ xrandr --output eDP1 --brightness -.5

(it was an accident, via a script triggered by a keyboard shortcut.)

my screen display went really bright. I saw that I made something wrong and
reset the brightness using

$ xrandr --output eDP1 --brightness 1

Unfortunately, my display died. for a few hours my display was still working,
with pixels failing. barely working I must say, until today. My screen died.
But that's ok, I'm not complaining with xrandr. It was my fault, and my fault
only.

I took some time to read the source code for xrandr and I think I found what
made this behavior occur. I presume it's a bug, and not an intended effect.

In the lines 2991-2999 of the file xrandr.c we can see the following:

if (!strcmp ("--brightness", argv[i])) {
if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
if (sscanf(argv[i], "%f", &config_output->brightness) != 1)
argerr ("%s: invalid argument '%s'\n", argv[i-1], argv[i]);
config_output->changes |= changes_gamma;
setit_1_2 = True;
continue;
}

And, in other lines we can see that the brightness variable is passed to some
math operations, like in the lines 1511-1532, where the variable is used to
compute gamma values.

I thing that, when a negative value is passed to such operations, the returned
value is NaN (actually the value is -nan). I didn't get much of the following
steps after that, but I suppose that using a NaN as a parameter to a function
that set gamma values for pixels (and a negative brightness to the screen) would
mess up with the display.

I'm definitely not a experienced programmer nor understand in great detail what
xrandr is actually doing, but I think that if we add a couple of more lines for
check input values this behavior/(bug?) would be solved.

What I could think of to add is this two commented lines in the middle of the
piece of code reproduced above:

if (!strcmp ("--brightness", argv[i])) {
if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
if (sscanf(argv[i], "%f", &config_output->brightness) != 1)
argerr ("%s: invalid argument '%s'\n", argv[i-1], argv[i]);
/* if (isnan(config_output->brightness) || isinf(config_output->brightness)) */
/* argerr ("%s: '%s' must be a positive value\n", argv[i-1], argv[i]); */
config_output->changes |= changes_gamma;
setit_1_2 = True;
continue;
}

I don't know if this would be accepted, but, anyway, I thank you all for your
attention!

PS: I'm using an updated Arch Linux, and xrandr version is 1.5.1

$ uname -a
Linux mcsarch 5.11.10-arch1-1 #1 SMP PREEMPT Fri, 26 Mar 2021 00:11:29 +0000 x86_64 GNU/Linux

$ xrandr --version
xrandr program version 1.5.1
Server reports RandR version 1.6
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg/attachments/20210329/25f6b5d4/attachment.htm>