Re: [PATCH 3/4] hw: Use qdev_get_parent_bus() in qdev_get_own_fw_dev_path_from_handler()

[Philippe Mathieu-Daudé]

On 14/2/23 00:29, Richard Henderson wrote:
> On 2/12/23 12:47, Philippe Mathieu-Daudé wrote:
> > -static char *qdev_get_fw_dev_path_from_handler(BusState *bus, 
> > DeviceState *dev)
> > +static char *qdev_get_fw_dev_path_from_handler(DeviceState *dev)
> >   {
> >       Object *obj = OBJECT(dev);
> > +    BusState *bus = qdev_get_parent_bus(dev);
> >       char *d = NULL;
> >       while (!d && obj->parent) {
>
> This is a separate change from...
>
> > -char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, 
> > DeviceState *dev)
> > +char *qdev_get_own_fw_dev_path_from_handler(DeviceState *dev)
> >   {
> >       Object *obj = OBJECT(dev);
> > -    return fw_path_provider_try_get_dev_path(obj, bus, dev);
> > +    return fw_path_provider_try_get_dev_path(obj, 
> > qdev_get_parent_bus(dev), dev);
>
> ... this, which is what $SUBJECT says.
>
> > @@ -67,7 +68,7 @@ static int qdev_get_fw_dev_path_helper(DeviceState 
> > *dev, char *p, int size)
> >       if (dev && dev->parent_bus) {
> >           char *d;
> >           l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, 
> > size);
> > -        d = qdev_get_fw_dev_path_from_handler(dev->parent_bus, dev);
> > +        d = qdev_get_fw_dev_path_from_handler(dev);
>
> We've already accessed parent_bus just above
>
> >           if (!d) {
> >               d = bus_get_fw_dev_path(dev->parent_bus, dev);
>
> ... and just below.  So, what's the cleanup?

qdev_get_own_fw_dev_path_from_handler() being a public API, I wanted to
clean it to avoid a funny case when it is called with
bus != qdev_get_parent_bus(dev). Maybe I merged 2 patches in one, I'll
revisit. Or I can just add assert(bus == qdev_get_parent_bus(dev)) to
prove the API is convoluted. I'll reword on before respin.