[Qemu-devel] [PATCH 0/1] usb-serial: Remove double call to qemu_chr_add_handlers( NULL )

[Qemu-devel] [PATCH 0/1] usb-serial: Remove double call to qemu_chr_add_handlers( NULL )

[Hans de Goede]

usb-serial has a qdev chardev property, and hw/qdev-properties-system.c
already contains:

static void release_chr(Object *obj, const char *name, void *opaque)
{
    DeviceState *dev = DEVICE(obj);
    Property *prop = opaque;
    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);

    if (*ptr) {
        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
    }
}

So doing the qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL); from
the usb handle_destroy function too will lead to it being done twice, which
will result in a wrong value for cs->avail_connections.

Note:
1) I noticed this will working on other stuff, but I've not actually seen this
happening (I did not try to trigger it), so please review carefully.

2) There are other places which are doing a qemu_chr_add_handlers( NULL )
too, but those don't use a qdev chardev property, so this does not apply:

backends/rng-egd.c
hw/serial.c
hw/xen_console.c

backends/rng-egd.c is weird / suspect here since it uses a qdev string
property for the chardev and then uses qemu_chr_find to get it. I wonder
why it is not simply using a chardev property rather then a string property?

Regards,

Hans

[Qemu-devel] [PATCH] usb-serial: Remove double call to qemu_chr_add_handlers( NULL )

[Hans de Goede]

usb-serial has a qdev chardev property, and hw/qdev-properties-system.c
already contains:

static void release_chr(Object *obj, const char *name, void *opaque)
{
    DeviceState *dev = DEVICE(obj);
    Property *prop = opaque;
    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);

    if (*ptr) {
        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
    }
}

So doing the qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL); from
the usb handle_destroy function too will lead to it being done twice, which
will result in a wrong value for cs->avail_connections.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/dev-serial.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 7c314dc..21ddef6 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -410,13 +410,6 @@ static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
     }
 }
 
-static void usb_serial_handle_destroy(USBDevice *dev)
-{
-    USBSerialState *s = (USBSerialState *)dev;
-
-    qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL);
-}
-
 static int usb_serial_can_read(void *opaque)
 {
     USBSerialState *s = opaque;
@@ -595,7 +588,6 @@ static void usb_serial_class_initfn(ObjectClass *klass, void *data)
     uc->handle_reset   = usb_serial_handle_reset;
     uc->handle_control = usb_serial_handle_control;
     uc->handle_data    = usb_serial_handle_data;
-    uc->handle_destroy = usb_serial_handle_destroy;
     dc->vmsd = &vmstate_usb_serial;
     dc->props = serial_properties;
 }
@@ -623,7 +615,6 @@ static void usb_braille_class_initfn(ObjectClass *klass, void *data)
     uc->handle_reset   = usb_serial_handle_reset;
     uc->handle_control = usb_serial_handle_control;
     uc->handle_data    = usb_serial_handle_data;
-    uc->handle_destroy = usb_serial_handle_destroy;
     dc->vmsd = &vmstate_usb_serial;
     dc->props = braille_properties;
 }
-- 
1.8.1.4