[systemd-devel] [PATCH udev v2] udev: Add program/rule to export touchscreen/tablet size as udev properties
carlosg at gnome.org
Fri Dec 19 09:30:55 PST 2014
On vie, 2014-12-19 at 17:20 +0100, Lennart Poettering wrote:
> 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 ())
Yup, that was an oversight, originally not in a #define at all.
> Please size these arrays with the right length. We have
> DECIMAL_STR_MAX(int) as a macro to get the right size.
Ah, cheers! didn't notice that one.
> > + _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)
Oh, makes sense.
Another update coming...
More information about the systemd-devel