Re: [Qemu-devel] [PATCH] s390x/sclpconsole: Remove dead code - make _error functions void

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 03/04/2018 02:45 PM, Nia Alarie wrote:
> These functions always return 0. By changing their return type to
> void, some dead code can be removed.

the event facility part looks ok, but I am asking myself if we should
go a step further.
Do we need the exit callback at all? We can certainly keep it for reasons of symmetry
but it looks like the other event handlers (quiesce and cpu) do not define it at all.
I addition to that, I have a hard time imagine a usecase for such an exit handler.

> 
> Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> ---
>  hw/char/sclpconsole-lm.c          | 3 +--
>  hw/char/sclpconsole.c             | 3 +--
>  hw/s390x/event-facility.c         | 6 +-----
[...]>  include/hw/s390x/event-facility.h | 2 +-
>  6 files changed, 8 insertions(+), 14 deletions(-)

> 
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index c500bdaf29..7a02db54b8 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c
> @@ -318,9 +318,8 @@ static int console_init(SCLPEvent *event)
>      return 0;
>  }
> 
> -static int console_exit(SCLPEvent *event)
> +static void console_exit(SCLPEvent *event)
>  {
> -    return 0;
>  }
> 
>  static void console_reset(DeviceState *dev)
> diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
> index d0265dfa7a..e2a80dd2dd 100644
> --- a/hw/char/sclpconsole.c
> +++ b/hw/char/sclpconsole.c
> @@ -246,9 +246,8 @@ static void console_reset(DeviceState *dev)
>     scon->notify = false;
>  }
> 
> -static int console_exit(SCLPEvent *event)
> +static void console_exit(SCLPEvent *event)
>  {
> -    return 0;
>  }
> 
>  static Property console_properties[] = {
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 155a69467b..4263d28012 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -436,11 +436,7 @@ static void event_unrealize(DeviceState *qdev, Error **errp)
>      SCLPEvent *event = SCLP_EVENT(qdev);
>      SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
>      if (child->exit) {
> -        int rc = child->exit(event);
> -        if (rc < 0) {
> -            error_setg(errp, "SCLP event exit failed.");
> -            return;
> -        }
> +        child->exit(event);
>      }
>  }
> 
[...]

>  /* Performance improves when virtqueue kick processing is decoupled from the
> diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
> index 5119b9b7f0..833f12a5e7 100644
> --- a/include/hw/s390x/event-facility.h
> +++ b/include/hw/s390x/event-facility.h
> @@ -174,7 +174,7 @@ typedef struct SCLPEvent {
>  typedef struct SCLPEventClass {
>      DeviceClass parent_class;
>      int (*init)(SCLPEvent *event);
> -    int (*exit)(SCLPEvent *event);
> +    void (*exit)(SCLPEvent *event);
> 
>      /* get SCLP's send mask */
>      unsigned int (*get_send_mask)(void);
>