Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

On Mon, Feb 22, 2016 at 07:06:34PM +0100, Jiri Pirko wrote:
> Mon, Feb 22, 2016 at 06:51:50PM CET, stefanha@gmail.com wrote:
> >On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
> >> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
> >>      /* allocate worlds */
> >>  
> >>      r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
> >> -    r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
> >> +
> >> +    if (!r->world_name) {
> >> +        r->world_name = g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
> >> +    }
> >> +
> >> +    r->world_dflt = rocker_world_type_by_name(r, r->world_name);
> >> +    if (!r->world_dflt) {
> >> +        fprintf(stderr,
> >> +                "rocker: requested world \"%s\" does not exist\n",
> >> +                r->world_name);
> >> +        return -EINVAL;
> >> +    }
> >
> >world_name is leaked here.  Please use goto to run the appropriate
> >cleanup code instead of returning directly.
> 
> I did the same what is done with "r->name = g_strdup(ROCKER)"
> 
> I assumed since this is a property, the caller core will take care of
> that.

You are right, the string properties will be freed by qdev property
release.

The return statement added by this patch still leaks
r->worlds[ROCKER_WORLD_TYPE_OF_DPA] so goto err_world_alloc is needed.

(pci_rocker_uninit() is not called if pci_rocker_init() fails.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]