| Message ID | 20230212224730.51438-4-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/qdev: Housekeeping around qdev_get_parent_bus() | expand |
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? r~
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.
diff --git a/hw/core/qdev-fw.c b/hw/core/qdev-fw.c index a31958355f..c2df1f4796 100644 --- a/hw/core/qdev-fw.c +++ b/hw/core/qdev-fw.c @@ -41,9 +41,10 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev) return NULL; } -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) { @@ -53,11 +54,11 @@ static char *qdev_get_fw_dev_path_from_handler(BusState *bus, DeviceState *dev) return d; } -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); } static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size) @@ -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); if (!d) { d = bus_get_fw_dev_path(dev->parent_bus, dev); } diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index f5b3b2f89a..93718be156 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -774,7 +774,7 @@ bool bus_is_in_reset(BusState *bus); BusState *sysbus_get_default(void); char *qdev_get_fw_dev_path(DeviceState *dev); -char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, DeviceState *dev); +char *qdev_get_own_fw_dev_path_from_handler(DeviceState *dev); void device_class_set_props(DeviceClass *dc, Property *props); diff --git a/softmmu/bootdevice.c b/softmmu/bootdevice.c index 2106f1026f..7834bf3333 100644 --- a/softmmu/bootdevice.c +++ b/softmmu/bootdevice.c @@ -214,7 +214,7 @@ static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes, if (!ignore_suffixes) { if (dev) { - d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev); + d = qdev_get_own_fw_dev_path_from_handler(dev); if (d) { assert(!suffix); s = d;
No need to pass 'dev' and 'dev->parent_bus' when we can retrieve 'parent_bus' with qdev_get_parent_bus(). Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/core/qdev-fw.c | 9 +++++---- include/hw/qdev-core.h | 2 +- softmmu/bootdevice.c | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-)