Re: [PATCH] of/irq: init struct resource to 0 in of_irq_to_resource()

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Fri, Jul 19, 2013 at 1:57 AM, Sebastian Andrzej Siewior
<bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> On 07/19/2013 04:49 AM, Rob Herring wrote:
>> On 07/18/2013 05:24 AM, Sebastian Andrzej Siewior wrote:
>>> It almost does not matter because most users use only the ->start member
>>> of the struct. However if this struct is passed to a platform device
>>> which is then added via platform_device_add() then the ->parent member is
>>> also used.
>>
>> Most users don't use the resource struct at all. The ones that do, all
>> seem to do a memset beforehand. So I think current behavior is correct.
>
> Seriously? That is your argument here? Most users use memset()
> beforehand and that makes it correct? Now look at this: you pass a
> pointer to fill out a struct and you don't do it properly.

>From a standpoint of initializing variables close to the point it is
allocated, yes.

> There is of_address_to_resource() which does a memset() why is okay
> here use memset()?

Fair enough. Consistency is important too. Rather than a memset, I'd
rather see the 3 remaining fields initialized and avoid setting most
of them twice.

Rob

>
>>
>> There are some occurrences that pass a resource in, but then don't
>> actually use the resource. Those we should fix.
>
>
>
>>
>> Rob
>>
>>>
>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>> ---
>>>  drivers/of/irq.c | 1 +
>>>  1 file changed, 1 insearch/powerpc/sysdev/mv64x60_dev.crtion(+)
>>>
>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>> index a3c1c5a..e0a72fb 100644
>>> --- a/drivers/of/irq.c
>>> +++ b/drivers/of/irq.c
>>> @@ -345,6 +345,7 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
>>>      if (r && irq) {
>>>              const char *name = NULL;
>>>
>>> +            memset(r, 0, sizeof(*r));
>>>              /*
>>>               * Get optional "interrupts-names" property to add a name
>>>               * to the resource.
>>>
>>
>
> Sebastian