Re:Re: [PATCH] powerpc: kernel: Change the order of of_node_put()

From: Liang He
Date: Sat Jun 18 2022 - 04:06:45 EST







在 2022-06-18 15:13:13,"Christophe Leroy" <christophe.leroy@xxxxxxxxxx> 写道:
>
>
>Le 17/06/2022 à 13:26, Liang He a écrit :
>> In add_pcspkr(), it is better to call of_node_put() after the
>> 'if(!np)' check.
>
>Why is it better ?
>
>
>
>/**
> * of_node_put() - Decrement refcount of a node
> * @node: Node to dec refcount, NULL is supported to simplify writing of
> * callers
> */
>void of_node_put(struct device_node *node)
>{
> if (node)
> kobject_put(&node->kobj);
>}
>EXPORT_SYMBOL(of_node_put);
>
>
>
>Christophe

Hi, Christophe.

Thanks for your reply and I want to have a discussion.

In my thought, xxx_put(pointer)'s semantic usually means
this reference has been used done and will not be used
anymore. Is this semantic more reasonable, right?

Besides, if the np is NULL, we can just return and save a cpu
time for the xxx_put() call.

Otherwise, I prefer to call it 'use(check)-after-put'.

In fact, I have meet many other 'use(check)-after-put' instances
after I send this patch-commit, so I am waiting for this
discussion.

This is just my thought, it may be wrong.

Anyway, thanks for your reply.

Liang

>
>
>>
>> Signed-off-by: Liang He <windhl@xxxxxxx>
>> ---
>> arch/powerpc/kernel/setup-common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index eb0077b302e2..761817d1f4db 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -563,9 +563,9 @@ static __init int add_pcspkr(void)
>> int ret;
>>
>> np = of_find_compatible_node(NULL, NULL, "pnpPNP,100");
>> - of_node_put(np);
>> if (!np)
>> return -ENODEV;
>> + of_node_put(np);
>>
>> pd = platform_device_alloc("pcspkr", -1);
>> if (!pd)