From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36060) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPeX-000150-1A for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:34:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RuPeO-000452-2g for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:34:44 -0500 Received: from mail-pz0-f45.google.com ([209.85.210.45]:58275) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPeN-00044l-NF for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:34:36 -0500 Received: by dadp14 with SMTP id p14so6491586dad.4 for ; Mon, 06 Feb 2012 06:34:34 -0800 (PST) Message-ID: <4F2FE4F6.2080901@codemonkey.ws> Date: Mon, 06 Feb 2012 08:34:30 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328342577-25732-1-git-send-email-pbonzini@redhat.com> <1328342577-25732-20-git-send-email-pbonzini@redhat.com> In-Reply-To: <1328342577-25732-20-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 19/27] qdev: remove parse/print methods for pointer properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 02/04/2012 02:02 AM, Paolo Bonzini wrote: > Pointer properties (except for PROP_PTR of course) should not need a > legacy counterpart. In the future, relative paths will ensure that > QEMU will support the same syntax as now for drives etc.. > > Signed-off-by: Paolo Bonzini Reviewed-by: Anthony Liguori Regards, Anthony Liguori > --- > hw/qdev-properties.c | 128 ++++++++++++++++++++++++++++---------------------- > 1 files changed, 72 insertions(+), 56 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 9a67cc5..67995a3 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -579,9 +579,8 @@ PropertyInfo qdev_prop_string = { > > /* --- drive --- */ > > -static int parse_drive(DeviceState *dev, Property *prop, const char *str) > +static int parse_drive(DeviceState *dev, const char *str, void **ptr) > { > - BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop); > BlockDriverState *bs; > > bs = bdrv_find(str); > @@ -603,35 +602,30 @@ static void free_drive(DeviceState *dev, Property *prop) > } > } > > -static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len) > +static const char *print_drive(void *ptr) > { > - BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop); > - return snprintf(dest, len, "%s", > - *ptr ? bdrv_get_device_name(*ptr) : ""); > + return bdrv_get_device_name(ptr); > } > > -static void get_generic(Object *obj, Visitor *v, void *opaque, > - const char *name, Error **errp) > +static void get_pointer(Object *obj, Visitor *v, Property *prop, > + const char *(*print)(void *ptr), > + const char *name, Error **errp) > { > DeviceState *dev = DEVICE(obj); > - Property *prop = opaque; > void **ptr = qdev_get_prop_ptr(dev, prop); > - char buffer[1024]; > - char *p = buffer; > + char *p; > > - buffer[0] = 0; > - if (*ptr) { > - prop->info->print(dev, prop, buffer, sizeof(buffer)); > - } > + p = (char *) (*ptr ? print(*ptr) : ""); > visit_type_str(v,&p, name, errp); > } > > -static void set_generic(Object *obj, Visitor *v, void *opaque, > +static void set_pointer(Object *obj, Visitor *v, Property *prop, > + int (*parse)(DeviceState *dev, const char *str, void **ptr), > const char *name, Error **errp) > { > DeviceState *dev = DEVICE(obj); > - Property *prop = opaque; > Error *local_err = NULL; > + void **ptr = qdev_get_prop_ptr(dev, prop); > char *str; > int ret; > > @@ -650,36 +644,45 @@ static void set_generic(Object *obj, Visitor *v, void *opaque, > error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str); > return; > } > - ret = prop->info->parse(dev, prop, str); > + ret = parse(dev, str, ptr); > error_set_from_qdev_prop_error(errp, ret, dev, prop, str); > g_free(str); > } > > +static void get_drive(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + get_pointer(obj, v, opaque, print_drive, name, errp); > +} > + > +static void set_drive(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + set_pointer(obj, v, opaque, parse_drive, name, errp); > +} > + > PropertyInfo qdev_prop_drive = { > .name = "drive", > .type = PROP_TYPE_DRIVE, > .size = sizeof(BlockDriverState *), > - .parse = parse_drive, > - .print = print_drive, > - .get = get_generic, > - .set = set_generic, > + .get = get_drive, > + .set = set_drive, > .free = free_drive, > }; > > /* --- character device --- */ > > -static int parse_chr(DeviceState *dev, Property *prop, const char *str) > +static int parse_chr(DeviceState *dev, const char *str, void **ptr) > { > - CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); > - > - *ptr = qemu_chr_find(str); > - if (*ptr == NULL) { > + CharDriverState *chr = qemu_chr_find(str); > + if (chr == NULL) { > return -ENOENT; > } > - if ((*ptr)->avail_connections< 1) { > + if (chr->avail_connections< 1) { > return -EEXIST; > } > - --(*ptr)->avail_connections; > + *ptr = chr; > + --chr->avail_connections; > return 0; > } > > @@ -693,62 +696,75 @@ static void free_chr(DeviceState *dev, Property *prop) > } > > > -static int print_chr(DeviceState *dev, Property *prop, char *dest, size_t len) > +static const char *print_chr(void *ptr) > { > - CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); > + CharDriverState *chr = ptr; > > - if (*ptr&& (*ptr)->label) { > - return snprintf(dest, len, "%s", (*ptr)->label); > - } else { > - return snprintf(dest, len, ""); > - } > + return chr->label ? chr->label : ""; > +} > + > +static void get_chr(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + get_pointer(obj, v, opaque, print_chr, name, errp); > +} > + > +static void set_chr(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + set_pointer(obj, v, opaque, parse_chr, name, errp); > } > > PropertyInfo qdev_prop_chr = { > .name = "chr", > .type = PROP_TYPE_CHR, > .size = sizeof(CharDriverState*), > - .parse = parse_chr, > - .print = print_chr, > - .get = get_generic, > - .set = set_generic, > + .get = get_chr, > + .set = set_chr, > .free = free_chr, > }; > > /* --- netdev device --- */ > > -static int parse_netdev(DeviceState *dev, Property *prop, const char *str) > +static int parse_netdev(DeviceState *dev, const char *str, void **ptr) > { > - VLANClientState **ptr = qdev_get_prop_ptr(dev, prop); > + VLANClientState *netdev = qemu_find_netdev(str); > > - *ptr = qemu_find_netdev(str); > - if (*ptr == NULL) > + if (netdev == NULL) { > return -ENOENT; > - if ((*ptr)->peer) { > + } > + if (netdev->peer) { > return -EEXIST; > } > + *ptr = netdev; > return 0; > } > > -static int print_netdev(DeviceState *dev, Property *prop, char *dest, size_t len) > +static const char *print_netdev(void *ptr) > { > - VLANClientState **ptr = qdev_get_prop_ptr(dev, prop); > + VLANClientState *netdev = ptr; > > - if (*ptr&& (*ptr)->name) { > - return snprintf(dest, len, "%s", (*ptr)->name); > - } else { > - return snprintf(dest, len, ""); > - } > + return netdev->name ? netdev->name : ""; > +} > + > +static void get_netdev(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + get_pointer(obj, v, opaque, print_netdev, name, errp); > +} > + > +static void set_netdev(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + set_pointer(obj, v, opaque, parse_netdev, name, errp); > } > > PropertyInfo qdev_prop_netdev = { > .name = "netdev", > .type = PROP_TYPE_NETDEV, > .size = sizeof(VLANClientState*), > - .parse = parse_netdev, > - .print = print_netdev, > - .get = get_generic, > - .set = set_generic, > + .get = get_netdev, > + .set = set_netdev, > }; > > /* --- vlan --- */