[systemd-devel] [RFC 06/12] gfx: add keyboard layer

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrmann at gmail.com) wrote:

> +
> +enum {
> +        SD_GFX__LED_NUML,
> +        SD_GFX__LED_CAPSL,
> +        SD_GFX__LED_SCROLLL,
> +        SD_GFX__LED_COUNT,
> +};

Double underscores?

> +
> +struct sd_gfx_kbd {
> +        char *name;
> +        struct libevdev *evdev;
> +        int fd;
> +        struct xkb_state *state;
> +        sd_event *ev;
> +        sd_event_source *fd_source;
> +        sd_event_source *timer;
> +        sd_gfx_kbd_event_fn event_fn;
> +        void *data;
> +        void *fn_data;
> +
> +        unsigned int delay;
> +        unsigned int rate;
> +
> +        unsigned int sym_count;
> +        sd_gfx_kbd_event event;
> +        sd_gfx_kbd_event repeat;
> +
> +        xkb_mod_index_t xkb_mods[SD_GFX__MOD_COUNT];
> +        xkb_led_index_t xkb_leds[SD_GFX__LED_COUNT];
> +
> +        unsigned int awake : 1;
> +        unsigned int need_sync : 1;
> +        unsigned int repeating : 1;

If these are bools, then make them bools! C99 bools (i.e. the type
"bool" from stdbool.h) are awesome for bit fields! [ Please use C99 bools
everywhere in internal code, and "int" as bool type for public APIs,
since that's what pre-C99 code usually did, despite the stupidity this
results in when people use bit fields ]

> +        if (r < 0) {
> +                if (r == -EACCES)
> +                        log_debug("kbd %s: EACCES on led-update", kbd->name);
> +                else
> +                        log_error("kbd %s: cannot update leds: %d",
> +                                  kbd->name, r);

Please do not log from libraries. (Well, except when that's the primary
purpose of your function, or when you do so on LOG_DEBUG level, since
that is not visible normally anyway.)

Lennart

-- 
Lennart Poettering, Red Hat