[systemd-devel] [PATCH udev v2] udev: Add program/rule to export touchscreen/tablet size as udev properties

[Lennart Poettering]

On Fri, 19.12.14 16:01, Carlos Garnacho (carlosg at gnome.org) wrote:

> +
> +#include <linux/input.h>
> +#include "udev.h"
> +
> +/* Resolution is defined to be in units/mm for ABS_X/Y */
> +#define ABS_SIZE_MM(absinfo) ((absinfo.maximum - absinfo.minimum) / absinfo.resolution)

Just out of principle: can you turn this into a function please? We
try to avoid function-like macros that evaluate passed arguments
multiple times, because of the risk of side effects.

(Also, for macros like this, please always enclose the arguments you
use in ())

> +#define BUF_LEN 10
> +
> +static void extract_info(struct udev_device *dev, const char *devpath, bool test)
> +{
> +        struct input_absinfo xabsinfo = { 0 }, yabsinfo = { 0 };

Why { 0 } instead of {}? I mean, the struct has more members anyway,
and 0 is the default anyway, if you don't specify anything.

> +        char width[BUF_LEN], height[BUF_LEN];

Please size these arrays with the right length. We have
DECIMAL_STR_MAX(int) as a macro to get the right size.

> +        _cleanup_close_ int fd = -1;
> +
> +        if ((fd = open(devpath, O_RDONLY)) < 0)
> +                return;

Please split this into two lines. Also out of principle, always use
O_CLOEXEC (see CODING_STYLE for details):

fd = open(devpath, O_RDONLY|O_CLOEXEC);
if (fd < 0) 
     return;

Lennart

-- 
Lennart Poettering, Red Hat