linux-kernel - Re: [PATCH 27/32] staging: gasket: remove else clause after return in if clause

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: <20180717103715.k3cu7jiv4333aq4k@mwanda>
Date:   Tue, 17 Jul 2018 13:37:15 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Todd Poynor <toddpoynor@...il.com>
Cc:     Rob Springer <rspringer@...gle.com>,
        John Joseph <jnjoseph@...gle.com>,
        Ben Chan <benchan@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, Zhongze Hu <frankhu@...omium.org>,
        linux-kernel@...r.kernel.org, Simon Que <sque@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Todd Poynor <toddpoynor@...gle.com>,
        Dmitry Torokhov <dtor@...omium.org>
Subject: Re: [PATCH 27/32] staging: gasket: remove else clause after return
 in if clause

On Mon, Jul 16, 2018 at 07:09:21PM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@...gle.com>
> 
> Else after return is unnecessary and may cause static code checkers to
> complain.
> 
> Reported-by: Guenter Roeck <groeck@...omium.org>
> Signed-off-by: Simon Que <sque@...omium.org>
> Signed-off-by: Todd Poynor <toddpoynor@...gle.com>
> ---
>  drivers/staging/gasket/gasket_page_table.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
> index 2a27db658a4e4..080bf78c22c3b 100644
> --- a/drivers/staging/gasket/gasket_page_table.c
> +++ b/drivers/staging/gasket/gasket_page_table.c
> @@ -598,9 +598,8 @@ bool gasket_page_table_is_dev_addr_bad(
>  	if (gasket_addr_is_simple(pg_tbl, dev_addr))
>  		return gasket_is_simple_dev_addr_bad(
>  			pg_tbl, dev_addr, num_pages);
> -	else
> -		return gasket_is_extended_dev_addr_bad(
> -			pg_tbl, dev_addr, num_pages);
> +	return gasket_is_extended_dev_addr_bad(
> +		pg_tbl, dev_addr, num_pages);

This isn't the normal way to break up a line.  It might even cause
problems if you run with checkpatch.pl --strict.  And anyway this fits
on one line:

	return gasket_is_extended_dev_addr_bad(pg_tbl, dev_addr, num_pages);

But normally we would do:

	return gasket_is_extended_dev_addr_bad(pg_tbl, dev_addr, num_pages,
					       wha'ever, blah, blah, blah);

There are times when that looks a bit ugly if the variable names are
long so we could do:

	return gasket_is_extended_dev_addr_bad(
				long_variable_name_asdfasdfaasdf,
				pg_tbl, dev_addr);

But generally try to get a couple tabs in from the start of the function
name.

regards,
dan carpenter