[Qemu-devel] [PATCH] qdev: Remove some non-run codes in qdev_walk_children().

[Qemu-devel] [PATCH] qdev: Remove some non-run codes in qdev_walk_children().

[Zhi Yong Wu]

As you have known, qdev_reset_one() forever return a ZERO value to its caller, so some branches can not be forever covered in qdev_walk_children().

I thought that the return value for dev->info->reset(dev) can be returned, but dev->info->reset(dev) is referring to a function with void type.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 hw/qdev.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 292b52f..cbc5e02 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -513,10 +513,7 @@ int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
     int err;
 
     if (devfn) {
-        err = devfn(dev, opaque);
-        if (err) {
-            return err;
-        }
+        devfn(dev, opaque);
     }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] qdev: Remove some non-run codes in qdev_walk_children().

[Anthony Liguori]

On 08/07/2011 11:15 PM, Zhi Yong Wu wrote:
> As you have known, qdev_reset_one() forever return a ZERO value to its caller, so some branches can not be forever covered in qdev_walk_children().
>
> I thought that the return value for dev->info->reset(dev) can be returned, but dev->info->reset(dev) is referring to a function with void type.
>
> Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com>

But the details of qdev_reset_one are irrelevant to 
qdev_walk_children().  There may be other functions that want to walk 
the tree and it's useful to be able to interrupt the tree transversal by 
returning a non-zero value.

Regards,

Anthony Liguori

> ---
>   hw/qdev.c |    5 +----
>   1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 292b52f..cbc5e02 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -513,10 +513,7 @@ int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
>       int err;
>
>       if (devfn) {
> -        err = devfn(dev, opaque);
> -        if (err) {
> -            return err;
> -        }
> +        devfn(dev, opaque);
>       }
>
>       QLIST_FOREACH(bus,&dev->child_bus, sibling) {

Re: [Qemu-devel] [PATCH] qdev: Remove some non-run codes in qdev_walk_children().

[Zhi Yong Wu]

On Fri, Aug 12, 2011 at 3:40 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 08/07/2011 11:15 PM, Zhi Yong Wu wrote:
>>
>> As you have known, qdev_reset_one() forever return a ZERO value to its
>> caller, so some branches can not be forever covered in qdev_walk_children().
>>
>> I thought that the return value for dev->info->reset(dev) can be returned,
>> but dev->info->reset(dev) is referring to a function with void type.
>>
>> Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com>
>
> But the details of qdev_reset_one are irrelevant to qdev_walk_children().
>  There may be other functions that want to walk the tree and it's useful to
>From The function reference flow, currently only qdev_reset_all and
qbus_walk_children invoke qdev_walk_children, and they both pass
qdev_reset_one() as devfn to qdev_walk_children().
> be able to interrupt the tree transversal by returning a non-zero value.
Yeah, It is. Maybe later some new functions will need this.
Anyway, thanks for your explain.
>
> Regards,
>
> Anthony Liguori
>
>> ---
>>  hw/qdev.c |    5 +----
>>  1 files changed, 1 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 292b52f..cbc5e02 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -513,10 +513,7 @@ int qdev_walk_children(DeviceState *dev,
>> qdev_walkerfn *devfn,
>>      int err;
>>
>>      if (devfn) {
>> -        err = devfn(dev, opaque);
>> -        if (err) {
>> -            return err;
>> -        }
>> +        devfn(dev, opaque);
>>      }
>>
>>      QLIST_FOREACH(bus,&dev->child_bus, sibling) {
>
>

-- 
Regards,

Zhi Yong Wu