Message ID | 1360096768-8863-9-git-send-email-lersek@redhat.com |
---|---|
State | New |
Headers | show |
Laszlo Ersek <lersek@redhat.com> writes: > Eliminate the "match" variable, and move the remaining locals to the > narrowest possible scope. Also rename parameter bus_typename to type. > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > hw/qdev-monitor.c | 35 ++++++++++++++++------------------- > 1 files changed, 16 insertions(+), 19 deletions(-) > > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > index da32763..64359ee 100644 > --- a/hw/qdev-monitor.c > +++ b/hw/qdev-monitor.c > @@ -289,38 +289,35 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem) > } > > static BusState *qbus_find_recursive(BusState *bus, const char *name, > - const char *bus_typename) > + const char *type) > { > - BusClass *bus_class = BUS_GET_CLASS(bus); > BusChild *kid; > - BusState *child, *ret; > - int match = 1; > > - if (name && (strcmp(bus->name, name) != 0)) { > - match = 0; > - } > - if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) { > - match = 0; > - } > - if ((bus_class->max_dev != 0) && (bus_class->max_dev <= bus->max_index)) { > + if ((name == NULL || strcmp(bus->name, name) == 0) && > + (type == NULL || object_dynamic_cast(OBJECT(bus), type) != NULL)) { > + BusClass *bus_class = BUS_GET_CLASS(bus); > + > + /* name (if any) and type (if any) match; check free slots */ > + if (bus_class->max_dev == 0 || bus->max_index < bus_class->max_dev) { > + return bus; > + } > + > + /* bus is full -- fatal error for search by name */ > if (name != NULL) { > - /* bus was explicitly specified: return an error. */ > qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full", > bus->name); > return NULL; > - } else { > - /* bus was not specified: try to find another one. */ > - match = 0; > } > } > - if (match) { > - return bus; > - } I'm not sure this is an improvement. Hairy before, hairy after. Maybe it becomes more of an improvement later in the series. > > QTAILQ_FOREACH(kid, &bus->children, sibling) { > DeviceState *dev = kid->child; > + BusState *child; > + > QLIST_FOREACH(child, &dev->child_bus, sibling) { > - ret = qbus_find_recursive(child, name, bus_typename); > + BusState *ret; > + > + ret = qbus_find_recursive(child, name, type); > if (ret) { > return ret; > } I doubt moving locals to the narrowest scope is worth the churn. Wait a second... code before this patch: static BusState *qbus_find_recursive(BusState *bus, const char *name, const char *bus_typename) { BusClass *bus_class = BUS_GET_CLASS(bus); BusChild *kid; BusState *child, *ret; int match = 1; if (name && (strcmp(bus->name, name) != 0)) { match = 0; } if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) { match = 0; } if ((bus_class->max_dev != 0) && (bus_class->max_dev <= bus->max_index)) { We get here when bus if full. if (name != NULL) { bus is full, and we're looking for a named bus. /* bus was explicitly specified: return an error. */ qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full", bus->name); return NULL; Error out on full bus, as long as we're looking for a named bus (name != NULL), whether the full bus matches name and type (match != 0) or not (match == 0). } else { /* bus was not specified: try to find another one. */ match = 0; } } if (match) { bus matches name and type, and is not full. return bus; } QTAILQ_FOREACH(kid, &bus->children, sibling) { DeviceState *dev = kid->child; QLIST_FOREACH(child, &dev->child_bus, sibling) { ret = qbus_find_recursive(child, name, bus_typename); if (ret) { return ret; } } } return NULL; } Code after this patch: static BusState *qbus_find_recursive(BusState *bus, const char *name, const char *type) { BusChild *kid; if ((name == NULL || strcmp(bus->name, name) == 0) && (type == NULL || object_dynamic_cast(OBJECT(bus), type) != NULL)) { We get here when bus matches name and type. BusClass *bus_class = BUS_GET_CLASS(bus); /* name (if any) and type (if any) match; check free slots */ if (bus_class->max_dev == 0 || bus->max_index < bus_class->max_dev) { bus matches name and type, and is not full. Same as before. return bus; } /* bus is full -- fatal error for search by name */ if (name != NULL) { bus matches name and type, is full, and we're looking for a named bus. qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full", bus->name); return NULL; Error out on full bus, as long as we're looking for a named bus (name != NULL), and the full bus matches name and type. Silent change! Intentional? } } QTAILQ_FOREACH(kid, &bus->children, sibling) { DeviceState *dev = kid->child; BusState *child; QLIST_FOREACH(child, &dev->child_bus, sibling) { BusState *ret; ret = qbus_find_recursive(child, name, type); if (ret) { return ret; } } } return NULL; }
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c index da32763..64359ee 100644 --- a/hw/qdev-monitor.c +++ b/hw/qdev-monitor.c @@ -289,38 +289,35 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem) } static BusState *qbus_find_recursive(BusState *bus, const char *name, - const char *bus_typename) + const char *type) { - BusClass *bus_class = BUS_GET_CLASS(bus); BusChild *kid; - BusState *child, *ret; - int match = 1; - if (name && (strcmp(bus->name, name) != 0)) { - match = 0; - } - if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) { - match = 0; - } - if ((bus_class->max_dev != 0) && (bus_class->max_dev <= bus->max_index)) { + if ((name == NULL || strcmp(bus->name, name) == 0) && + (type == NULL || object_dynamic_cast(OBJECT(bus), type) != NULL)) { + BusClass *bus_class = BUS_GET_CLASS(bus); + + /* name (if any) and type (if any) match; check free slots */ + if (bus_class->max_dev == 0 || bus->max_index < bus_class->max_dev) { + return bus; + } + + /* bus is full -- fatal error for search by name */ if (name != NULL) { - /* bus was explicitly specified: return an error. */ qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full", bus->name); return NULL; - } else { - /* bus was not specified: try to find another one. */ - match = 0; } } - if (match) { - return bus; - } QTAILQ_FOREACH(kid, &bus->children, sibling) { DeviceState *dev = kid->child; + BusState *child; + QLIST_FOREACH(child, &dev->child_bus, sibling) { - ret = qbus_find_recursive(child, name, bus_typename); + BusState *ret; + + ret = qbus_find_recursive(child, name, type); if (ret) { return ret; }
Eliminate the "match" variable, and move the remaining locals to the narrowest possible scope. Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- hw/qdev-monitor.c | 35 ++++++++++++++++------------------- 1 files changed, 16 insertions(+), 19 deletions(-)