[Qemu-devel] [PATCH] hw: make *_exit functions return void

[Qemu-devel] [PATCH] hw: make *_exit functions return void

[Zihan Yang]

    According to the second suggestion listed on Dead code Removal
    of BiteSizedTasks, I modify some *_exit functions to make them
    return void. Some *_exit and *_exitfn functions that already
    return void remain unchanged. Also, the checks for the callers
    are deleted.

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/audio/hda-codec.c              | 3 +--
 hw/audio/intel-hda.c              | 3 +--
 hw/audio/intel-hda.h              | 2 +-
 hw/char/sclpconsole-lm.c          | 3 +--
 hw/char/sclpconsole.c             | 3 +--
 hw/core/qdev.c                    | 6 +-----
 hw/s390x/event-facility.c         | 6 +-----
 hw/s390x/virtio-ccw.c             | 7 +++----
 hw/s390x/virtio-ccw.h             | 2 +-
 hw/usb/dev-smartcard-reader.c     | 3 +--
 include/hw/qdev-core.h            | 2 +-
 include/hw/s390x/event-facility.h | 2 +-
 12 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
     return 0;
 }
 
-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
     HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
         }
     }
     AUD_remove_card(&a->card);
-    return 0;
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 537face..991c704 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -70,7 +70,7 @@ static void hda_codec_dev_realize(DeviceState *qdev, Error **errp)
     }
 }
 
-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_exit(DeviceState *qdev)
 {
     HDACodecDevice *dev = HDA_CODEC_DEVICE(qdev);
     HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -78,7 +78,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
     if (cdc->exit) {
         cdc->exit(dev);
     }
-    return 0;
 }
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
     DeviceClass parent_class;
 
     int (*init)(HDACodecDevice *dev);
-    int (*exit)(HDACodecDevice *dev);
+    void (*exit)(HDACodecDevice *dev);
     void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
     void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool output);
 } HDACodecDeviceClass;
diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 07d6ebd..6fa8922 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 b78f240..d9bbc5e 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/core/qdev.c b/hw/core/qdev.c
index 1e7fb33..5415843 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -238,11 +238,7 @@ static void device_unrealize(DeviceState *dev, Error **errp)
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
     if (dc->exit) {
-        int rc = dc->exit(dev);
-        if (rc < 0) {
-            error_setg(errp, "Device exit failed.");
-            return;
-        }
+        dc->exit(dev);
     }
 }
 
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 34b2faf..f7c509c 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -413,11 +413,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 00b3bde..d1c4ff6 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -722,7 +722,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     }
 }
 
-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;
@@ -735,7 +735,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)
@@ -1616,12 +1615,12 @@ static void virtio_ccw_busdev_realize(DeviceState *dev, Error **errp)
     virtio_ccw_device_realize(_dev, errp);
 }
 
-static int virtio_ccw_busdev_exit(DeviceState *dev)
+static void 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);
 }
 
 static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 41d4010..ce8baa3 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -74,7 +74,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/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 757b8b3..71e3667 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1289,7 +1289,7 @@ void ccid_card_card_inserted(CCIDCardState *card)
     ccid_on_slot_change(s, true);
 }
 
-static int ccid_card_exit(DeviceState *qdev)
+static void ccid_card_exit(DeviceState *qdev)
 {
     CCIDCardState *card = CCID_CARD(qdev);
     USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
@@ -1300,7 +1300,6 @@ static int ccid_card_exit(DeviceState *qdev)
     }
     ccid_card_exitfn(card);
     s->card = NULL;
-    return 0;
 }
 
 static int ccid_card_init(DeviceState *qdev)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b44b476..0efd9e9 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -31,7 +31,7 @@ typedef enum DeviceCategory {
 } DeviceCategory;
 
 typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
index def1bb0..1a32f3a 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -162,7 +162,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.7.4

Re: [Qemu-devel] [PATCH] hw: make *_exit functions return void

[Cornelia Huck]

On Sun, 16 Apr 2017 20:26:01 +0800
Zihan Yang <tgnyang@gmail.com> wrote:

>     According to the second suggestion listed on Dead code Removal
>     of BiteSizedTasks, I modify some *_exit functions to make them
>     return void. Some *_exit and *_exitfn functions that already
>     return void remain unchanged. Also, the checks for the callers
>     are deleted.
> 
> Signed-off-by: Zihan Yang <tgnyang@gmail.com>
> ---
>  hw/audio/hda-codec.c              | 3 +--
>  hw/audio/intel-hda.c              | 3 +--
>  hw/audio/intel-hda.h              | 2 +-
>  hw/char/sclpconsole-lm.c          | 3 +--
>  hw/char/sclpconsole.c             | 3 +--
>  hw/core/qdev.c                    | 6 +-----
>  hw/s390x/event-facility.c         | 6 +-----
>  hw/s390x/virtio-ccw.c             | 7 +++----
>  hw/s390x/virtio-ccw.h             | 2 +-
>  hw/usb/dev-smartcard-reader.c     | 3 +--
>  include/hw/qdev-core.h            | 2 +-
>  include/hw/s390x/event-facility.h | 2 +-
>  12 files changed, 14 insertions(+), 28 deletions(-)

Hm, I wondered why I had a deja vu... ah, here:
https://marc.info/?t=149003291200008&r=1&w=2

The author of that patch did not follow up AFAICS, but some of the
suggestions made in that thread apply to your patch as well.

In particular:


> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index 07d6ebd..6fa8922 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 b78f240..d9bbc5e 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;
>  }

These two now empty functions are not very useful; it is better to
simply remove them.

With that change, I'd take a split-out patch for sclp and virtio-ccw
through my tree, or I'd ack a not-split-out version.

Re: [Qemu-devel] [PATCH] hw: make *_exit functions return void

[Zihan Yang]

(I forgot to reply all in last email, so I resend the reply again)
I have resubmitted two split-out patches in https://lists.nongnu.org/
archive/html/qemu-devel/2017-04/msg03096.html, which change the exit
funtion of sclp and virtio-ccw. But the exit function of DeviceClass still
returns int after the patch because it involves other folders like hw/audio,
and as suggested by Markus Armbruster, the exit function of DeviceClass
should be converted to unrealize(), so I just leave `virtio_ccw_busdev_exit`
at present because it is an exit function of DeviceClass.


2017-04-19 0:15 GMT+08:00 Cornelia Huck <cornelia.huck@de.ibm.com>:

> On Sun, 16 Apr 2017 20:26:01 +0800
> Zihan Yang <tgnyang@gmail.com> wrote:
>
> >     According to the second suggestion listed on Dead code Removal
> >     of BiteSizedTasks, I modify some *_exit functions to make them
> >     return void. Some *_exit and *_exitfn functions that already
> >     return void remain unchanged. Also, the checks for the callers
> >     are deleted.
> >
> > Signed-off-by: Zihan Yang <tgnyang@gmail.com>
> > ---
> >  hw/audio/hda-codec.c              | 3 +--
> >  hw/audio/intel-hda.c              | 3 +--
> >  hw/audio/intel-hda.h              | 2 +-
> >  hw/char/sclpconsole-lm.c          | 3 +--
> >  hw/char/sclpconsole.c             | 3 +--
> >  hw/core/qdev.c                    | 6 +-----
> >  hw/s390x/event-facility.c         | 6 +-----
> >  hw/s390x/virtio-ccw.c             | 7 +++----
> >  hw/s390x/virtio-ccw.h             | 2 +-
> >  hw/usb/dev-smartcard-reader.c     | 3 +--
> >  include/hw/qdev-core.h            | 2 +-
> >  include/hw/s390x/event-facility.h | 2 +-
> >  12 files changed, 14 insertions(+), 28 deletions(-)
>
> Hm, I wondered why I had a deja vu... ah, here:
> https://marc.info/?t=149003291200008&r=1&w=2
>
> The author of that patch did not follow up AFAICS, but some of the
> suggestions made in that thread apply to your patch as well.
>
> In particular:
>
>
> > diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> > index 07d6ebd..6fa8922 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 b78f240..d9bbc5e 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;
> >  }
>
> These two now empty functions are not very useful; it is better to
> simply remove them.
>
> With that change, I'd take a split-out patch for sclp and virtio-ccw
> through my tree, or I'd ack a not-split-out version.
>
>