diff mbox series

net: gro: use IS_ERR before PTR_ERR

Message ID 20211207073116.3856-1-guozhengkui@vivo.com (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series net: gro: use IS_ERR before PTR_ERR | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 20 this patch: 20
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 14 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Guo Zhengkui Dec. 7, 2021, 7:31 a.m. UTC
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 <guozhengkui@vivo.com>
---
 net/core/gro.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Alexander Lobakin Dec. 7, 2021, 2:41 p.m. UTC | #1
From: Guo Zhengkui <guozhengkui@vivo.com>
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 <guozhengkui@vivo.com>
> ---
>  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
Guo Zhengkui Dec. 8, 2021, 7:09 a.m. UTC | #2
On 2021/12/7 22:41, Alexander Lobakin wrote:
> From: Guo Zhengkui <guozhengkui@vivo.com>
> 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 <guozhengkui@vivo.com>
>> ---
>>   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
>
diff mbox series

Patch

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;