Re: [Qemu-devel] [PATCH v2 05/21] char: add a /chardevs container

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

On Mon, Feb 27, 2017 at 8:01 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

>
>
> On 27/02/2017 14:41, Marc-André Lureau wrote:
> >
> > -    chr = CHARDEV(object_new(typename));
> > +    if (id) {
> > +        obj = object_new_with_props(typename, get_chardevs_root(),
> > +                                    id, &local_err, NULL);
> > +    } else {
> > +        obj = object_new(typename);
> > +    }
>
> I think you should open code object_new_with_props.  This can be just
> object_new(typename)...
>
>
Yes, it simplies things. Done. Any other remark about the rest of the
series? probably not worth a resend yet.

thanks


> > +    if (local_err) {
> > +        assert(!obj);
> > +        goto end;
> > +    }
> > +
> > +    chr = CHARDEV(obj);
> >      chr->label = g_strdup(id);
> >
> >      qemu_char_open(chr, backend, &be_opened, &local_err);
> >      if (local_err) {
> > -        error_propagate(errp, local_err);
> > -        object_unref(OBJECT(chr));
> > -        return NULL;
> > +        goto end;
> >      }
> >
> >      if (!chr->filename) {
> > @@ -1250,6 +1263,18 @@ Chardev *qemu_chardev_new(const char *id, const
> char *typename,
> >          qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> >      }
> >
> > +end:
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        if (chr) {
> > +            if (id) {
> > +                object_unparent(OBJECT(chr));
> > +            } else {
> > +                object_unref(OBJECT(chr));
> > +            }
>
> Then here you do not have a parent object and you can do only unref...
>
> > +        }
> > +        return NULL;
> > +    }
>
> ... while here you do the object_property_add_child+object_unref.  The
> object_property_add_child keeps the object alive, while object_unref
> drops the reference originally returned by object_new.
>
> Paolo
>
> --
Marc-André Lureau