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

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

[Nia Alarie]

These functions always return 0. By changing their return type to
void, some dead code can be removed.

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 +-----
 hw/s390x/virtio-ccw.c             | 6 +++---
 hw/s390x/virtio-ccw.h             | 2 +-
 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);
     }
 }
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 8f7fbc2ab7..0773818a82 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -752,7 +752,7 @@ out_err:
     g_free(sch);
 }
 
-static int virtio_ccw_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_exit(VirtioCcwDevice *dev)
 {
     CcwDevice *ccw_dev = CCW_DEVICE(dev);
     SubchDev *sch = ccw_dev->sch;
@@ -765,7 +765,6 @@ static int virtio_ccw_exit(VirtioCcwDevice *dev)
         release_indicator(&dev->routes.adapter, dev->indicators);
         dev->indicators = NULL;
     }
-    return 0;
 }
 
 static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
@@ -1710,7 +1709,8 @@ static int virtio_ccw_busdev_exit(DeviceState *dev)
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
     VirtIOCCWDeviceClass *_info = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
 
-    return _info->exit(_dev);
+    _info->exit(_dev);
+    return 0;
 }
 
 static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 3905f3a3d6..8ffe7fe095 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -76,7 +76,7 @@ typedef struct VirtioCcwDevice VirtioCcwDevice;
 typedef struct VirtIOCCWDeviceClass {
     CCWDeviceClass parent_class;
     void (*realize)(VirtioCcwDevice *dev, Error **errp);
-    int (*exit)(VirtioCcwDevice *dev);
+    void (*exit)(VirtioCcwDevice *dev);
 } VirtIOCCWDeviceClass;
 
 /* 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);
-- 
2.16.2

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

[Christian Borntraeger]

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);
> 

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

[Cornelia Huck]

On Mon, 5 Mar 2018 09:07:17 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> 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.

Agreed. I think we should just remove them.

[I dimly remember having that discussion before...]