linux-kernel - Re: [PATCH] bcache: Use #ifdef instead of boolean variable

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]

Date:   Sat, 10 Oct 2020 01:00:48 +0200
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     Alex Dewar <alex.dewar90@...il.com>
Cc:     Coly Li <colyli@...e.de>,
        Kent Overstreet <kent.overstreet@...il.com>,
        linux-bcache@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bcache: Use #ifdef instead of boolean variable

On 09/10/2020 20.34, Alex Dewar wrote:
> The variable async_registration is used to indicate whether or not
> CONFIG_BCACHE_ASYNC_REGISTRATION is enabled, triggering a (false)
> warning in Coverity about unreachable code. However, it seems clearer in
> this case just to use an #ifdef, so let's do that instead.
> 
> Addresses-Coverity-ID: 1497746: Control flow issues (DEADCODE)

I think that coverity check needs to be ignored. The kernel is full of
things that are supposed to be eliminated by the compiler, but still
checked for valid syntax etc. Often it's even more hidden than this,
something like

// some header
#ifdef CONFIG_FOO
int foo(void);
#else
static inline int foo(void) { return 0; }
#endif

// code

  if (foo()) { ... // this goes away for CONFIG_FOO=n }

> Signed-off-by: Alex Dewar <alex.dewar90@...il.com>
> ---
>  drivers/md/bcache/super.c | 40 +++++++++++++++++----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 46a00134a36a..6d4127881c6a 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2504,11 +2504,6 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  	struct cache_sb_disk *sb_disk;
>  	struct block_device *bdev;
>  	ssize_t ret;
> -	bool async_registration = false;
> -
> -#ifdef CONFIG_BCACHE_ASYNC_REGISTRATION
> -	async_registration = true;
> -#endif

If anything, this should simply be changed to

  bool async_registration = IS_ENABLED(CONFIG_BCACHE_ASYNC_REGISTRATION);

Rasmus