linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Nikolay Borisov <nborisov@suse.com>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	David Sterba <dsterba@suse.cz>,
	linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 1/3] btrfs: remove pointless out label in find_first_block_group
Date: Wed, 27 May 2020 11:31:50 +0200	[thread overview]
Message-ID: <20200527093150.GZ18421@twin.jikos.cz> (raw)
In-Reply-To: <fce3d7f8-23b8-b417-f5d3-3f6af7738118@suse.com>

On Tue, May 26, 2020 at 06:24:49PM +0300, Nikolay Borisov wrote:
> 
> 
> On 26.05.20 г. 17:21 ч., Johannes Thumshirn wrote:
> > The 'out' label in find_first_block_group() does not do anything in terms
> > of cleanup.
> > 
> > It is better to directly return 'ret' instead of jumping to out to not
> > confuse readers. Additionally there is no need to initialize ret with 0.
> > 
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> I personally prefer returning fast aka the way you've done it but dunno
> if David is a fan of this. In any case:

I'm not and the pattern that should be used is a mix of both. The first
part is for the 'obvious' cases:

Early exit from function can use return, eg. when a feature is not
enabled, when there's no cleanup needed, when the return is inside an if
and is not nested

For the rest it's recommended to use the goto and single return as it
may be a big chunk of code with a lot of nesting and a return somewhere
in the middle reads harder.

In example of find_first_block_group the first 'goto out' after
btrfs_search_slot would be a candidate to convert to return, but the
rest is inside a while loop so goto is preferred.

It is also important to keep one style and switch to it eventually and I
think that the goto + single return is quite common nowadays, exceptions
exist in the old code.

  reply	other threads:[~2020-05-27  9:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-26 14:21 [PATCH 0/3] 3 small cleanups for find_first_block_group Johannes Thumshirn
2020-05-26 14:21 ` [PATCH 1/3] btrfs: remove pointless out label in find_first_block_group Johannes Thumshirn
2020-05-26 15:24   ` Nikolay Borisov
2020-05-27  9:31     ` David Sterba [this message]
2020-05-26 14:21 ` [PATCH 2/3] btrfs: get mapping tree directly from fsinfo " Johannes Thumshirn
2020-05-26 15:25   ` Nikolay Borisov
2020-05-26 14:21 ` [PATCH 3/3] btrfs: factor out reading of bg from find_frist_block_group Johannes Thumshirn
2020-05-26 15:33   ` Nikolay Borisov
2020-05-27  7:46     ` Johannes Thumshirn

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=20200527093150.GZ18421@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=nborisov@suse.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;
as well as URLs for NNTP newsgroup(s).