[PATCH libevdev] Reset the struct on set_fd

[David Herrmann]

Hi Peter

On Tue, Oct 22, 2013 at 5:11 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> libevdev_set_fd may fail at a number of points. If it does, it errors out
> but does not unset previously set fields. Thus, a client may call set_fd
> again for the same struct but on a different fd and have it succeed.
> Depending on when set_fd bailed out the first time, some fields may already
> be set.
>
> Thus, reset the whole struct at set_fd time to make sure we're nulled out
> appropriately.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  libevdev/libevdev.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index 6f203e2..4c1f089 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -110,20 +110,28 @@ log_msg(enum libevdev_log_priority priority,
>         va_end(args);
>  }
>
> -LIBEVDEV_EXPORT struct libevdev*
> -libevdev_new(void)
> +static void
> +libevdev_reset(struct libevdev *dev)
>  {
> -       struct libevdev *dev;
> -
> -       dev = calloc(1, sizeof(*dev));
> -       if (!dev)
> -               return NULL;
> +       memset(dev, 0, sizeof(*dev));
>         dev->fd = -1;
>         dev->initialized = false;
>         dev->num_slots = -1;
>         dev->current_slot = -1;
>         dev->grabbed = LIBEVDEV_UNGRAB;
>         dev->sync_state = SYNC_NONE;
> +}
> +
> +LIBEVDEV_EXPORT struct libevdev*
> +libevdev_new(void)
> +{
> +       struct libevdev *dev;
> +
> +       dev = malloc(sizeof(*dev));
> +       if (!dev)
> +               return NULL;
> +
> +       libevdev_reset(dev);
>
>         return dev;
>  }
> @@ -212,6 +220,8 @@ libevdev_set_fd(struct libevdev* dev, int fd)
>         } else if (fd < 0)
>                 return -EBADF;
>
> +       libevdev_reset(dev);
> +

I think we should call libevdev_reset() in the error-path, too.
Otherwise, the state might be initialized with partial values and
*_get_*() functions will return partial results. But it's a nit-pick
as we don't guarantee anything about the state after a failed
set_fd(). So I'm fine with either way:

Reviewed-by: David Herrmann <dh.herrmann at gmail.com>

Thanks
David

>         rc = ioctl(fd, EVIOCGBIT(0, sizeof(dev->bits)), dev->bits);
>         if (rc < 0)
>                 goto out;
> --
> 1.8.3.1
>