Re: [PATCH-for-5.0] ui/input-linux: Do not ignore ioctl() return value

[Darren Kenny]

Philippe Mathieu-Daudé writes:

> Fix warnings reported by Clang static code analyzer:
>
>     CC      ui/input-linux.o
>       ui/input-linux.c:343:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:351:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:354:13: warning: Value stored to 'rc' is never read
>               rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
>               ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:357:13: warning: Value stored to 'rc' is never read
>               rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
>               ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:365:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:366:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Darren Kenny <address@hidden>

> ---
>  ui/input-linux.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/ui/input-linux.c b/ui/input-linux.c
> index a7b280b25b..ef37b14d6f 100644
> --- a/ui/input-linux.c
> +++ b/ui/input-linux.c
> @@ -334,13 +334,15 @@ static void input_linux_complete(UserCreatable *uc, 
> Error **errp)
>  
>      rc = ioctl(il->fd, EVIOCGBIT(0, sizeof(evtmap)), &evtmap);
>      if (rc < 0) {
> -        error_setg(errp, "%s: failed to read event bits", il->evdev);
> -        goto err_close;
> +        goto err_read_event_bits;
>      }
>  
>      if (evtmap & (1 << EV_REL)) {
>          relmap = 0;
>          rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          if (relmap & (1 << REL_X)) {
>              il->has_rel_x = true;
>          }
> @@ -349,12 +351,25 @@ static void input_linux_complete(UserCreatable *uc, 
> Error **errp)
>      if (evtmap & (1 << EV_ABS)) {
>          absmap = 0;
>          rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          if (absmap & (1 << ABS_X)) {
>              il->has_abs_x = true;
>              rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute X value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>              il->abs_x_min = absinfo.minimum;
>              il->abs_x_max = absinfo.maximum;
>              rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute Y value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>              il->abs_y_min = absinfo.minimum;
>              il->abs_y_max = absinfo.maximum;
>          }
> @@ -363,7 +378,14 @@ static void input_linux_complete(UserCreatable *uc, 
> Error **errp)
>      if (evtmap & (1 << EV_KEY)) {
>          memset(keymap, 0, sizeof(keymap));
>          rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
> +        if (rc < 0) {
> +            error_setg(errp, "%s: failed to get global key state", 
> il->evdev);
> +            goto err_close;
> +        }
>          for (i = 0; i < KEY_CNT; i++) {
>              if (keymap[i / 8] & (1 << (i % 8))) {
>                  if (linux_is_button(i)) {
> @@ -390,6 +412,9 @@ static void input_linux_complete(UserCreatable *uc, Error 
> **errp)
>      il->initialized = true;
>      return;
>  
> +err_read_event_bits:
> +    error_setg(errp, "%s: failed to read event bits", il->evdev);
> +
>  err_close:
>      close(il->fd);
>      return;