linux-kernel - Re: [PATCH] btrfs: Turn an 'else if' into an 'else' in btrfs_uuid_tree

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190307175550.GO31119@twin.jikos.cz>
Date:   Thu, 7 Mar 2019 18:55:50 +0100
From:   David Sterba <dsterba@...e.cz>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH] btrfs: Turn an 'else if' into an 'else' in
 btrfs_uuid_tree_add

On Thu, Mar 07, 2019 at 09:35:15AM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
> 
> fs/btrfs/uuid-tree.c:129:13: warning: variable 'eb' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> fs/btrfs/uuid-tree.c:129:13: warning: variable 'offset' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> 
> Clang can't tell that all cases are covered with this final else if.

The chain of conditions is

if (ret >= 0)
else if (ret == -EEXIST)
else if (ret < 0)

I'd think that compiler should be able to somehow represent the coverage
of the ranges and report if it's not complete and leading to some
uninitialized variables.

What if the last condition is 'ret < -EINVAL' obscuring the numerical
value, it'd be easy to miss that there are 20 unhandled values. In such
case the final 'else' would be the right thing to do.

Anyway, I'll apply the patch because it makes the code easier to follow.
Thanks.