Re: [cocci] [PATCH] scripts/coccinelle/free: add NULL test before dev_{put, hold} functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> I find the patch subject improvable.
> You would like to detect the remaining usage of redundant null pointer checks
> before selected function calls.
> Thus it would be nice if the corresponding description can become clearer
> another bit, wouldn't it?

Sorry, I did not get your mean. Did you mean I should point out the selected function?

And the following is one warning, what would be like as your suggestion?

./drivers/infiniband/core/lag.c:98:2-10: WARNING: NULL check before dev_{put, hold} functions is not needed.

>
>
>> Since commit b37a46683739 ("netdevice: add the case if dev is NULL"),
>> NULL check before dev_{put, hold} functions is not needed.
>>
>> Signed-off-by: Ziyang Xuan <william.xuanziyang@xxxxxxxxxx>
>> ---
>>  scripts/coccinelle/free/ifnulldev_put.cocci | 54 +++++++++++++++++++++
>
> How do you think about to achieve further collateral evolution?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b37a466837393af72fe8bcb8f1436410f3f173f3
>
> Will it become feasible to combine any more source code analysis approaches?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/free/ifnullfree.cocci?id=8c23f235a6a8ae43abea215812eb9d8cf4dd165e#n2
>
>
>> +// Comments: -
>
> Can such a line be omitted?
>
It's ok.
>
>> +@r2 depends on patch@
>> +expression E;
>> +@@
>> +- if (E != NULL)
>> +(
>> +  __dev_put(E);
>> +|
>> +  dev_put(E);
>> +|
>> +  dev_put_track(E, ...);
>> +|
>> +  __dev_hold(E);
>> +|
>> +  dev_hold(E);
>> +|
>> +  dev_hold_track(E, ...);
>> +)
>
> Did you order the case distinctions in the SmPL disjunction according to
> the call frequencies of the mentioned function names?

No, no any special, just list the related functions.

>
> Otherwise, I imagine that another software design option can be considered for
> the application of nested disjunctions by the means of the semantic patch language.
>
> Code variant:
>
> @r2 depends on patch@
> expression E;
> @@
> -if (E != NULL)
> (
> (__dev_put
> |dev_put
> |__dev_hold
> |dev_hold
> )(E)
> |
> (dev_put_track
> |dev_hold_track
> )(E, ...)
> )
> ;
It's ok.
>
> Regards,
> Markus
>
> .



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux