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 <[email protected]>
Signed-off-by: Jiapeng Chong <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Jiapeng Chong <[email protected]>
Reviewed-by: Joseph Qi <[email protected]>
> ---
> 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];
> }
>
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 <[email protected]> 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 <[email protected]>
> Signed-off-by: Jiapeng Chong <[email protected]>
> ---
> 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
> [email protected]
> https://oss.oracle.com/mailman/listinfo/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