qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH qemu v10] spapr: Implement Open Firmware client interface


From: Greg Kurz
Subject: Re: [PATCH qemu v10] spapr: Implement Open Firmware client interface
Date: Mon, 7 Dec 2020 10:53:36 +0100

On Mon, 7 Dec 2020 18:33:34 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

[...]

> >> +    }
> >> +
> >> +    return offset;
> >> +}
> >> +
> >> +static uint32_t of_client_finddevice(const void *fdt, uint32_t nodeaddr)
> >> +{
> >> +    char *node, *unit, *part;
> > 
> > If you do this:
> > 
> >      g_autofree *node = NULL, *unit = NULL, *part = NULL;
> 
> 
> Did you mean
>         g_autofree char *node = NULL, *unit = NULL, *part = NULL;
> ?
> 

Yes. Sorry for the missing char :)

[...]

> >> +static uint32_t spapr_of_client_open(SpaprMachineState *spapr, const char 
> >> *path)
> >> +{
> >> +    int offset;
> >> +    uint32_t ret = 0;
> >> +    SpaprOfInstance *inst = NULL;
> >> +    char *node, *unit, *part;
> >> +
> >> +    if (spapr->of_instance_last == 0xFFFFFFFF) {
> >> +        /* We do not recycle ihandles yet */
> >> +        goto trace_exit;
> > 
> > And g_free() is passed uninitialized pointers.
> > 
> > A typical use case for the g_auto magic.
> 
> g_autofree, you mean?
> 

If you switch to g_strsplit_set(), you'd need a g_auto(GStrv). But since you
explained that you'd rather keep split_path() as is, then you should use
g_autofree indeed.

[...]

> 
> Thanks! I'll repost in a sec. But I still wonder on what terms this is 
> going to be allowed in the QEMU tree at all.
> 

Pros:

This notably improves the boot time of the -kernel/-initrd experience.
Even if the feature isn't used in production, it can be quite useful for
developers.

Cons:

It doesn't provide a full alternative to SLOF with respect to the boot
loader.

This is still a lot of code that we'll need to support, especially since
the changes affect paths used in production.



IMHO, unless this gets momentum in the community, and eventually
allows to get rid of SLOF once and for all, it seems premature to
merge this.

Cheers,

--
Greg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]