The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	Tom Rix <trix@redhat.com>,
	ntfs3@lists.linux.dev, llvm@lists.linux.dev,
	patches@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next v2 1/2] fs/ntfs3: Don't use uni1 uninitialized in ntfs_d_compare()
Date: Fri, 21 Oct 2022 10:10:55 -0700	[thread overview]
Message-ID: <Y1LSn+O44lCRR3qE@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20221004232359.285685-1-nathan@kernel.org>

Ping? This is still breaking our builds in -next...

On Tue, Oct 04, 2022 at 04:23:58PM -0700, Nathan Chancellor wrote:
> Clang warns:
> 
>   fs/ntfs3/namei.c:445:7: error: variable 'uni1' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>                   if (toupper(c1) != toupper(c2)) {
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
>   ./include/linux/ctype.h:64:20: note: expanded from macro 'toupper'
>   #define toupper(c) __toupper(c)
>                     ^
>   fs/ntfs3/namei.c:487:12: note: uninitialized use occurs here
>           __putname(uni1);
>                     ^~~~
>   ./include/linux/fs.h:2789:65: note: expanded from macro '__putname'
>   #define __putname(name)         kmem_cache_free(names_cachep, (void *)(name))
>                                                                         ^~~~
>   fs/ntfs3/namei.c:445:3: note: remove the 'if' if its condition is always false
>                   if (toupper(c1) != toupper(c2)) {
>                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   fs/ntfs3/namei.c:434:7: error: variable 'uni1' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>                   if (!lm--) {
>                       ^~~~~
>   fs/ntfs3/namei.c:487:12: note: uninitialized use occurs here
>           __putname(uni1);
>                     ^~~~
>   ./include/linux/fs.h:2789:65: note: expanded from macro '__putname'
>   #define __putname(name)         kmem_cache_free(names_cachep, (void *)(name))
>                                                                         ^~~~
>   fs/ntfs3/namei.c:434:3: note: remove the 'if' if its condition is always false
>                   if (!lm--) {
>                   ^~~~~~~~~~~~
>   fs/ntfs3/namei.c:430:22: note: initialize the variable 'uni1' to silence this warning
>           struct cpu_str *uni1, *uni2;
>                               ^
>                               = NULL
>   2 errors generated.
> 
> There is no point in calling __putname() in these particular error
> paths, as there has been no corresponding __getname() call yet. Just
> return directly in these blocks to clear up the warning.
> 
> Fixes: a3a956c78efa ("fs/ntfs3: Add option "nocase"")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1729
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> 
> v2:
>     * Pick up Nick's reviewed-by tag.
> v1: https://lore.kernel.org/20221004144145.1345772-1-nathan@kernel.org/
> 
>  fs/ntfs3/namei.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
> index 315763eb05ff..5d3a6ce3f05f 100644
> --- a/fs/ntfs3/namei.c
> +++ b/fs/ntfs3/namei.c
> @@ -431,10 +431,8 @@ static int ntfs_d_compare(const struct dentry *dentry, unsigned int len1,
>  
>  	/* First try fast implementation. */
>  	for (;;) {
> -		if (!lm--) {
> -			ret = len1 == len2 ? 0 : 1;
> -			goto out;
> -		}
> +		if (!lm--)
> +			return len1 == len2 ? 0 : 1;
>  
>  		if ((c1 = *n1++) == (c2 = *n2++))
>  			continue;
> @@ -442,10 +440,8 @@ static int ntfs_d_compare(const struct dentry *dentry, unsigned int len1,
>  		if (c1 >= 0x80 || c2 >= 0x80)
>  			break;
>  
> -		if (toupper(c1) != toupper(c2)) {
> -			ret = 1;
> -			goto out;
> -		}
> +		if (toupper(c1) != toupper(c2))
> +			return 1;
>  	}
>  
>  	/*
> 
> base-commit: d45da67caedacd500879de5e649360cc70777af7
> -- 
> 2.37.3
> 
> 

  parent reply	other threads:[~2022-10-21 17:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-04 23:23 [PATCH -next v2 1/2] fs/ntfs3: Don't use uni1 uninitialized in ntfs_d_compare() Nathan Chancellor
2022-10-04 23:23 ` [PATCH -next v2 2/2] fs/ntfs3: Eliminate unnecessary ternary operator " Nathan Chancellor
2022-10-04 23:26   ` Nick Desaulniers
2022-11-12 18:11   ` Konstantin Komarov
2022-10-21 17:10 ` Nathan Chancellor [this message]
2022-10-26 16:56   ` [PATCH -next v2 1/2] fs/ntfs3: Don't use uni1 uninitialized " Konstantin Komarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1LSn+O44lCRR3qE@dev-arch.thelio-3990X \
    --to=nathan@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=ndesaulniers@google.com \
    --cc=ntfs3@lists.linux.dev \
    --cc=patches@lists.linux.dev \
    --cc=trix@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox