Re: [Xen-devel] [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing

[Artem Mygaiev <artem_mygaiev@xxxxxxxx>]

Hi Julien


On 06.12.16 16:19, Julien Grall wrote:
> Hi Artem,
>
> On 06/12/16 13:57, Artem Mygaiev wrote:
>> Remove unused tnode variables in device tree IRQ map parsing
>
> This description is slightly wrong, the variable are used but not the value.
True
>
>> Coverity-ID 1381866, 1381867
>>
>> Signed-off-by: Artem Mygaiev <artem_mygaiev@xxxxxxxx>
>
> This code is from Linux (see drivers/of/irq.c) and I would prefer to keep it 
> very similar as long as it does not lead to a bug:
I understand, but in the kernel code tnode is used to safely decrement kobj 
refcount (e.g. in 4.8.12):

 144                tnode = of_get_parent(old);
 145                of_node_put(old);
 146                old = tnode;

AFAIU in Xen refcounts are dropped, so probably tnode can be dropped as a part 
of the infrastructure... no?

> Nacked-by: Julien Grall <julien.grall@xxxxxxx>
>
>> ---
>>  xen/common/device_tree.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index 1be074b..edc48f6 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -1053,7 +1053,7 @@ int dt_for_each_irq_map(const struct dt_device_node 
>> *dev,
>>                                    void *),
>>                          void *data)
>>  {
>> -    const struct dt_device_node *ipar, *tnode, *old = NULL;
>> +    const struct dt_device_node *ipar, *old = NULL;
>>      const __be32 *tmp, *imap;
>>      u32 intsize = 1, addrsize, pintsize = 0, paddrsize = 0;
>>      u32 imaplen;
>> @@ -1078,7 +1078,6 @@ int dt_for_each_irq_map(const struct dt_device_node 
>> *dev,
>>              intsize = be32_to_cpu(*tmp);
>>              break;
>>          }
>> -        tnode = ipar;
>>          ipar = dt_irq_find_parent(ipar);
>>      } while ( ipar );
>>      if ( ipar == NULL )
>> @@ -1101,8 +1100,7 @@ int dt_for_each_irq_map(const struct dt_device_node 
>> *dev,
>>      old = ipar;
>>      do {
>>          tmp = dt_get_property(old, "#address-cells", NULL);
>> -        tnode = dt_get_parent(old);
>> -        old = tnode;
>> +        old = dt_get_parent(old);
>>      } while ( old && tmp == NULL );
>>
>>      old = NULL;
>> @@ -1238,7 +1236,7 @@ static int dt_irq_map_raw(const struct dt_device_node 
>> *parent,
>>                            const __be32 *addr,
>>                            struct dt_raw_irq *oirq)
>>  {
>> -    const struct dt_device_node *ipar, *tnode, *old = NULL, *newpar = NULL;
>> +    const struct dt_device_node *ipar, *old = NULL, *newpar = NULL;
>>      const __be32 *tmp, *imap, *imask;
>>      u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
>>      u32 imaplen;
>> @@ -1261,7 +1259,6 @@ static int dt_irq_map_raw(const struct dt_device_node 
>> *parent,
>>              intsize = be32_to_cpu(*tmp);
>>              break;
>>          }
>> -        tnode = ipar;
>>          ipar = dt_irq_find_parent(ipar);
>>      } while ( ipar );
>>      if ( ipar == NULL )
>> @@ -1281,8 +1278,7 @@ static int dt_irq_map_raw(const struct dt_device_node 
>> *parent,
>>      old = ipar;
>>      do {
>>          tmp = dt_get_property(old, "#address-cells", NULL);
>> -        tnode = dt_get_parent(old);
>> -        old = tnode;
>> +        old = dt_get_parent(old);
>>      } while ( old && tmp == NULL );
>>
>>      old = NULL;
>>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel