On 31/01/2013 16:52, Peter Maydell wrote:
On 31 January 2013 15:42, Laszlo Ersek <ler...@redhat.com> wrote:
The bus we're looking for could be in the sub-tree rooted at the node
being checked; don't skip looping over the children.
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
hw/qdev-monitor.c | 10 +--------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 4e2a92b..34f5014 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -295,15 +295,7 @@ static BusState *qbus_find_recursive(BusState *bus, const
char *name,
match = 0;
}
if ((bus_class->max_dev != 0) && (bus_class->max_dev <= bus->max_index)) {
- 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;
- }
+ match = 0;
}
This looks like the wrong fix to this problem -- if the user passed
us a specific name to search for and we found it and it was full, then
we definitely want to stop here. On the other hand, if the user passed
us a specific name and this bus isn't that named bus then we shouldn't
be checking the max_index at all.
So I think the right fix is that the condition should be
if (match && (bus_class->max_dev != 0)
&& (bus_class->max_dev <= bus->max_index)) {
and the rest of the code in this function stays as is.
-- PMM
The final result looks the same no?
If you look the qdev-monitor.c code just above the patch, you'll find
the same thing:
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)) {
...
Fred
