[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 08/15] qbus_find_recursive(): reorganize
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 08/15] qbus_find_recursive(): reorganize |
Date: |
Wed, 20 Mar 2013 15:44:22 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Laszlo Ersek <address@hidden> 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 <address@hidden>
> ---
> 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;
}
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 08/15] qbus_find_recursive(): reorganize,
Markus Armbruster <=