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.
(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
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
