2021-12-07 07:31:38

by Guo Zhengkui

Subject: [PATCH] net: gro: use IS_ERR before PTR_ERR

fix following cocci warning:
./net/core/gro.c:493:5-12: ERROR: PTR_ERR applied after initialization to constant on line 441

Signed-off-by: Guo Zhengkui <[email protected]>
---
net/core/gro.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/core/gro.c b/net/core/gro.c
index 8ec8b44596da..ee08f7b23793 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -490,9 +490,11 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
if (&ptype->list == head)
goto normal;

- if (PTR_ERR(pp) == -EINPROGRESS) {
- ret = GRO_CONSUMED;
- goto ok;
+ if (IS_ERR(pp)) {
+ if (PTR_ERR(pp) == -EINPROGRESS) {
+ ret = GRO_CONSUMED;
+ goto ok;
+ }
}

same_flow = NAPI_GRO_CB(skb)->same_flow;
--
2.20.1

2021-12-07 14:41:58

by Alexander Lobakin

Subject: Re: [PATCH] net: gro: use IS_ERR before PTR_ERR

From: Guo Zhengkui <[email protected]>
Date: Tue, 7 Dec 2021 15:31:09 +0800

Hi, thanks for your patch.

> fix following cocci warning:
> ./net/core/gro.c:493:5-12: ERROR: PTR_ERR applied after initialization to constant on line 441
>
> Signed-off-by: Guo Zhengkui <[email protected]>
> ---
> net/core/gro.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/gro.c b/net/core/gro.c
> index 8ec8b44596da..ee08f7b23793 100644
> --- a/net/core/gro.c
> +++ b/net/core/gro.c
> @@ -490,9 +490,11 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
> if (&ptype->list == head)
> goto normal;
>
> - if (PTR_ERR(pp) == -EINPROGRESS) {
> - ret = GRO_CONSUMED;
> - goto ok;
> + if (IS_ERR(pp)) {
> + if (PTR_ERR(pp) == -EINPROGRESS) {
> + ret = GRO_CONSUMED;
> + goto ok;
> + }
> }

`if (PTR_ERR(ptr) == -ERRNO)` itself is correct without a check for
IS_ERR(). The former basically is a more precise test comparing to
the latter.
Not sure if compilers can get it well, but in ideal case the first
will be omitted from the object code at all, and so do we.

In case I'm wrong and this is a correct fix, it at least shouldn't
increase the indentation by one, these two conditions can be placed
into one `if` statement.

NAK.

>
> same_flow = NAPI_GRO_CB(skb)->same_flow;
> --
> 2.20.1

Al

2021-12-08 07:09:54

by Guo Zhengkui

Subject: Re: [PATCH] net: gro: use IS_ERR before PTR_ERR

On 2021/12/7 22:41, Alexander Lobakin wrote:
> From: Guo Zhengkui <[email protected]>
> Date: Tue, 7 Dec 2021 15:31:09 +0800
>
> Hi, thanks for your patch.
>
>> fix following cocci warning:
>> ./net/core/gro.c:493:5-12: ERROR: PTR_ERR applied after initialization to constant on line 441
>>
>> Signed-off-by: Guo Zhengkui <[email protected]>
>> ---
>> net/core/gro.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/core/gro.c b/net/core/gro.c
>> index 8ec8b44596da..ee08f7b23793 100644
>> --- a/net/core/gro.c
>> +++ b/net/core/gro.c
>> @@ -490,9 +490,11 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
>> if (&ptype->list == head)
>> goto normal;
>>
>> - if (PTR_ERR(pp) == -EINPROGRESS) {
>> - ret = GRO_CONSUMED;
>> - goto ok;
>> + if (IS_ERR(pp)) {
>> + if (PTR_ERR(pp) == -EINPROGRESS) {
>> + ret = GRO_CONSUMED;
>> + goto ok;
>> + }
>> }
>
> `if (PTR_ERR(ptr) == -ERRNO)` itself is correct without a check for
> IS_ERR(). The former basically is a more precise test comparing to
> the latter.

Yes, even without `IS_ERR`, it runs well.

At least, `IS_ERR` before `PTR_ERR` is a good habit. :)

Zhengkui

> Not sure if compilers can get it well, but in ideal case the first
> will be omitted from the object code at all, and so do we.
>
> In case I'm wrong and this is a correct fix, it at least shouldn't
> increase the indentation by one, these two conditions can be placed
> into one `if` statement.
>
> NAK.
>
>>
>> same_flow = NAPI_GRO_CB(skb)->same_flow;
>> --
>> 2.20.1
>
> Al
>