linux-kernel - Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check

Open Source and information security mailing list archives

Message-ID: <1EED85DE-EFE1-4FBA-9AA5-648D8E586DD9@oracle.com>
Date:   Mon, 10 May 2021 15:50:46 +0000
From:   Wengang Wang <wen.gang.wang@...cle.com>
To:     Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>
CC:     "mark@...heh.com" <mark@...heh.com>,
        "ocfs2-devel@....oracle.com" <ocfs2-devel@....oracle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Ocfs2-devel] [PATCH v2] ocfs2: Reomve err less than zero check

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@...ux.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@...ux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@...ux.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@....oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives