Re: [hackers] [slstatus][PATCH] backlight: implemented openbsd support

From: lucas <lucas_AT_nanashi.co>
Date: Wed, 23 May 2018 15:57:37 -0300

Hi Tobias,

Two notes below

> diff --git a/components/backlight.c b/components/backlight.c
> index f9c4096..21e06a1 100644
> --- a/components/backlight.c
> +++ b/components/backlight.c
> _AT_@ -29,4 +29,30 @@
>
> return bprintf("%d", cur * 100 / max);
> }
> +
> +#elif defined(__OpenBSD__)
> + #include <fcntl.h>
> + #include <sys/ioctl.h>
> + #include <sys/time.h>
> + #include <dev/wscons/wsconsio.h>
> +
> + const char *
> + backlight_perc(const char *unused)
> + {
> + int fd, err;
> + struct wsdisplay_param wsd_param = {
> + .param = WSDISPLAYIO_PARAM_BRIGHTNESS
> + };
> +
> + if ((fd = open("/dev/ttyC0", O_RDONLY)) < 0) {

This only works if you run startx from tty. If not, /dev/ttyC0 would be 0600
owned by root.

> + warn("could not open /dev/ttyC0");
> + return NULL;
> + }
> + if ((err = ioctl(fd, WSDISPLAYIO_GETPARAM, &wsd_param)) < 0) {

Why storing the return value in err if it isn't used?

> + warn("ioctl 'WSDISPLAYIO_GETPARAM' failed");
> + return NULL;
> + }
> + return bprintf("%d", wsd_param.curval * 100 / wsd_param.max);

wsd_param also includes a .min field. It should be properly scaled instead
of just dividing by .max

> + }
> +
> #endif
> diff --git a/config.def.h b/config.def.h
> index 75debe5..3a0f838 100644
> --- a/config.def.h
> +++ b/config.def.h
> _AT_@ -14,6 +14,7 @@ static const char unknown_str[] = "n/a";
> *
> * backlight_perc backlight percentage device name
> * (intel_backlight)
> + * NULL on OpenBSD
> * battery_perc battery percentage battery name (BAT0)
> * NULL on OpenBSD
> * battery_state battery charging state battery name (BAT0)
> --
> 2.16.2
>
>

Cheers.
Received on Wed May 23 2018 - 20:57:37 CEST