Hey Lennart, On vie, 2014-12-19 at 17:20 +0100, Lennart Poettering wrote: > On Fri, 19.12.14 16:01, Carlos Garnacho (carl...@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.
Aha sure. > > (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. <snip> > 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) > return; Oh, makes sense. Another update coming... Cheers, Carlos _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
