All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ocfs2: Reomve err less than zero check
@ 2021-05-10 10:37 ` Jiapeng Chong
  0 siblings, 0 replies; 8+ messages in thread
From: Jiapeng Chong @ 2021-05-10 10:37 UTC (permalink / raw)
  To: mark; +Cc: jlbec, joseph.qi, ocfs2-devel, linux-kernel, Jiapeng Chong

Because enum dlm_status starts from 0, the check for err < 0 is always
false.

Clean up the following coccicheck warning:

fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
less than zero.

fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
less than zero.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
Changes in v2:
  -Update commit log.

 fs/ocfs2/dlm/dlmdebug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
index d442cf5..817914d 100644
--- a/fs/ocfs2/dlm/dlmdebug.c
+++ b/fs/ocfs2/dlm/dlmdebug.c
@@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
 
 const char *dlm_errmsg(enum dlm_status err)
 {
-	if (err >= DLM_MAXSTATS || err < 0)
+	if (err >= DLM_MAXSTATS)
 		return dlm_errmsgs[DLM_MAXSTATS];
 	return dlm_errmsgs[err];
 }
@@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
 
 const char *dlm_errname(enum dlm_status err)
 {
-	if (err >= DLM_MAXSTATS || err < 0)
+	if (err >= DLM_MAXSTATS)
 		return dlm_errnames[DLM_MAXSTATS];
 	return dlm_errnames[err];
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
@ 2021-05-10 10:37 ` Jiapeng Chong
  0 siblings, 0 replies; 8+ messages in thread
From: Jiapeng Chong @ 2021-05-10 10:37 UTC (permalink / raw)
  To: mark; +Cc: Jiapeng Chong, ocfs2-devel, linux-kernel

Because enum dlm_status starts from 0, the check for err < 0 is always
false.

Clean up the following coccicheck warning:

fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
less than zero.

fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
less than zero.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
Changes in v2:
  -Update commit log.

 fs/ocfs2/dlm/dlmdebug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
index d442cf5..817914d 100644
--- a/fs/ocfs2/dlm/dlmdebug.c
+++ b/fs/ocfs2/dlm/dlmdebug.c
@@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
 
 const char *dlm_errmsg(enum dlm_status err)
 {
-	if (err >= DLM_MAXSTATS || err < 0)
+	if (err >= DLM_MAXSTATS)
 		return dlm_errmsgs[DLM_MAXSTATS];
 	return dlm_errmsgs[err];
 }
@@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
 
 const char *dlm_errname(enum dlm_status err)
 {
-	if (err >= DLM_MAXSTATS || err < 0)
+	if (err >= DLM_MAXSTATS)
 		return dlm_errnames[DLM_MAXSTATS];
 	return dlm_errnames[err];
 }
-- 
1.8.3.1


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] ocfs2: Reomve err less than zero check
  2021-05-10 10:37 ` [Ocfs2-devel] " Jiapeng Chong
@ 2021-05-10 11:33   ` Joseph Qi
  -1 siblings, 0 replies; 8+ messages in thread
From: Joseph Qi @ 2021-05-10 11:33 UTC (permalink / raw)
  To: Jiapeng Chong, mark, akpm; +Cc: jlbec, ocfs2-devel, linux-kernel



On 5/10/21 6:37 PM, Jiapeng Chong wrote:
> Because enum dlm_status starts from 0, the check for err < 0 is always
> false.
> 
> Clean up the following coccicheck warning:
> 
> fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
> less than zero.
> 
> fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
> less than zero.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> Changes in v2:
>   -Update commit log.
> 
>  fs/ocfs2/dlm/dlmdebug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
> index d442cf5..817914d 100644
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
>  
>  const char *dlm_errmsg(enum dlm_status err)
>  {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
>  		return dlm_errmsgs[DLM_MAXSTATS];
>  	return dlm_errmsgs[err];
>  }
> @@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
>  
>  const char *dlm_errname(enum dlm_status err)
>  {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
>  		return dlm_errnames[DLM_MAXSTATS];
>  	return dlm_errnames[err];
>  }
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
@ 2021-05-10 11:33   ` Joseph Qi
  0 siblings, 0 replies; 8+ messages in thread
From: Joseph Qi @ 2021-05-10 11:33 UTC (permalink / raw)
  To: Jiapeng Chong, mark, akpm; +Cc: ocfs2-devel, linux-kernel



On 5/10/21 6:37 PM, Jiapeng Chong wrote:
> Because enum dlm_status starts from 0, the check for err < 0 is always
> false.
> 
> Clean up the following coccicheck warning:
> 
> fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
> less than zero.
> 
> fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
> less than zero.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> Changes in v2:
>   -Update commit log.
> 
>  fs/ocfs2/dlm/dlmdebug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
> index d442cf5..817914d 100644
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
>  
>  const char *dlm_errmsg(enum dlm_status err)
>  {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
>  		return dlm_errmsgs[DLM_MAXSTATS];
>  	return dlm_errmsgs[err];
>  }
> @@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
>  
>  const char *dlm_errname(enum dlm_status err)
>  {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
>  		return dlm_errnames[DLM_MAXSTATS];
>  	return dlm_errnames[err];
>  }
> 

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
  2021-05-10 10:37 ` [Ocfs2-devel] " Jiapeng Chong
@ 2021-05-10 15:50   ` Wengang Wang
  -1 siblings, 0 replies; 8+ messages in thread
From: Wengang Wang @ 2021-05-10 15:50 UTC (permalink / raw)
  To: Jiapeng Chong; +Cc: mark, ocfs2-devel, linux-kernel

Hi Jiapeng,

Though the type of enum dlm_status has a value range starting from zero,  It can be assigned with negative values without warnings.
I am wondering why you are sure it can’t be negative?  You went over all the calling path and so you are sure it can’t be negative?
If you did that, I’d think/guess you should also be able to say it can’t be DLM_MAXSTATS or bigger, right? If not, which calling path are returning >= DLM_MAXSTATS value?
we should fix that too.

I’d think we should treat the two conditions of (err >= DLM_MAXSTATS) and the (err < 0) the same way.  If keep, keep both. if remove, remove both.
I’m suspecting the coccicheck warning is based on the type of enum (starting with 0), but if that’s the only theory to remove (err < 0), It’s NAK from me.

thanks,
wengang

> On May 10, 2021, at 3:37 AM, Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:
> 
> Because enum dlm_status starts from 0, the check for err < 0 is always
> false.
> 
> Clean up the following coccicheck warning:
> 
> fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
> less than zero.
> 
> fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
> less than zero.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
> Changes in v2:
>  -Update commit log.
> 
> fs/ocfs2/dlm/dlmdebug.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
> index d442cf5..817914d 100644
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
> 
> const char *dlm_errmsg(enum dlm_status err)
> {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
> 		return dlm_errmsgs[DLM_MAXSTATS];
> 	return dlm_errmsgs[err];
> }
> @@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
> 
> const char *dlm_errname(enum dlm_status err)
> {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
> 		return dlm_errnames[DLM_MAXSTATS];
> 	return dlm_errnames[err];
> }
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
@ 2021-05-10 15:50   ` Wengang Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Wengang Wang @ 2021-05-10 15:50 UTC (permalink / raw)
  To: Jiapeng Chong; +Cc: linux-kernel, ocfs2-devel

Hi Jiapeng,

Though the type of enum dlm_status has a value range starting from zero,  It can be assigned with negative values without warnings.
I am wondering why you are sure it can’t be negative?  You went over all the calling path and so you are sure it can’t be negative?
If you did that, I’d think/guess you should also be able to say it can’t be DLM_MAXSTATS or bigger, right? If not, which calling path are returning >= DLM_MAXSTATS value?
we should fix that too.

I’d think we should treat the two conditions of (err >= DLM_MAXSTATS) and the (err < 0) the same way.  If keep, keep both. if remove, remove both.
I’m suspecting the coccicheck warning is based on the type of enum (starting with 0), but if that’s the only theory to remove (err < 0), It’s NAK from me.

thanks,
wengang

> On May 10, 2021, at 3:37 AM, Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:
> 
> Because enum dlm_status starts from 0, the check for err < 0 is always
> false.
> 
> Clean up the following coccicheck warning:
> 
> fs/ocfs2/dlm/dlmdebug.c:222 dlm_errname() warn: unsigned 'err' is never
> less than zero.
> 
> fs/ocfs2/dlm/dlmdebug.c:214 dlm_errmsg() warn: unsigned 'err' is never
> less than zero.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
> Changes in v2:
>  -Update commit log.
> 
> fs/ocfs2/dlm/dlmdebug.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
> index d442cf5..817914d 100644
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -211,7 +211,7 @@ void dlm_print_one_lock(struct dlm_lock *lockid)
> 
> const char *dlm_errmsg(enum dlm_status err)
> {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
> 		return dlm_errmsgs[DLM_MAXSTATS];
> 	return dlm_errmsgs[err];
> }
> @@ -219,7 +219,7 @@ const char *dlm_errmsg(enum dlm_status err)
> 
> const char *dlm_errname(enum dlm_status err)
> {
> -	if (err >= DLM_MAXSTATS || err < 0)
> +	if (err >= DLM_MAXSTATS)
> 		return dlm_errnames[DLM_MAXSTATS];
> 	return dlm_errnames[err];
> }
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
  2021-05-10 15:50   ` Wengang Wang
@ 2021-05-11  2:16     ` Joseph Qi
  -1 siblings, 0 replies; 8+ messages in thread
From: Joseph Qi @ 2021-05-11  2:16 UTC (permalink / raw)
  To: Wengang Wang, Jiapeng Chong; +Cc: linux-kernel, ocfs2-devel, akpm




On 5/10/21 11:50 PM, Wengang Wang wrote:
> Hi Jiapeng,
> 
> Though the type of enum dlm_status has a value range starting from zero,  It can be assigned with negative values without warnings.
> I am wondering why you are sure it can’t be negative?  You went over all the calling path and so you are sure it can’t be negative?
> If you did that, I’d think/guess you should also be able to say it can’t be DLM_MAXSTATS or bigger, right? If not, which calling path are returning >= DLM_MAXSTATS value?
> we should fix that too.
> 
> I’d think we should treat the two conditions of (err >= DLM_MAXSTATS) and the (err < 0) the same way.  If keep, keep both. if remove, remove both.
> I’m suspecting the coccicheck warning is based on the type of enum (starting with 0), but if that’s the only theory to remove (err < 0), It’s NAK from me
DLM_MAXSTATS is a valid value for dlm_status.
Take a look again, I agree with Wengang that we'd better keep the check
here for those misused return code.

Thanks,
Joseph 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check
@ 2021-05-11  2:16     ` Joseph Qi
  0 siblings, 0 replies; 8+ messages in thread
From: Joseph Qi @ 2021-05-11  2:16 UTC (permalink / raw)
  To: Wengang Wang, Jiapeng Chong; +Cc: linux-kernel, ocfs2-devel




On 5/10/21 11:50 PM, Wengang Wang wrote:
> Hi Jiapeng,
> 
> Though the type of enum dlm_status has a value range starting from zero,  It can be assigned with negative values without warnings.
> I am wondering why you are sure it can’t be negative?  You went over all the calling path and so you are sure it can’t be negative?
> If you did that, I’d think/guess you should also be able to say it can’t be DLM_MAXSTATS or bigger, right? If not, which calling path are returning >= DLM_MAXSTATS value?
> we should fix that too.
> 
> I’d think we should treat the two conditions of (err >= DLM_MAXSTATS) and the (err < 0) the same way.  If keep, keep both. if remove, remove both.
> I’m suspecting the coccicheck warning is based on the type of enum (starting with 0), but if that’s the only theory to remove (err < 0), It’s NAK from me
DLM_MAXSTATS is a valid value for dlm_status.
Take a look again, I agree with Wengang that we'd better keep the check
here for those misused return code.

Thanks,
Joseph 

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-11  2:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 10:37 [PATCH v2] ocfs2: Reomve err less than zero check Jiapeng Chong
2021-05-10 10:37 ` [Ocfs2-devel] " Jiapeng Chong
2021-05-10 11:33 ` Joseph Qi
2021-05-10 11:33   ` [Ocfs2-devel] " Joseph Qi
2021-05-10 15:50 ` Wengang Wang
2021-05-10 15:50   ` Wengang Wang
2021-05-11  2:16   ` Joseph Qi
2021-05-11  2:16     ` Joseph Qi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.