Re: [PATCH] btrfs: send: Simplify send_create_inode_if_needed

[Marcos Paulo de Souza <mpdesouza@suse.de>]

On Mon, 2021-08-02 at 10:39 +0300, Nikolay Borisov wrote:
> 
> On 2.08.21 г. 2:35, Marcos Paulo de Souza wrote:
> > The out label is being overused, we can simply return if the
> > condition
> > permits.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> > ---
> >  fs/btrfs/send.c | 15 ++++-----------
> >  1 file changed, 4 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index 75cff564dedf..17cd67e41d3a 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -2727,19 +2727,12 @@ static int
> > send_create_inode_if_needed(struct send_ctx *sctx)
> >  	if (S_ISDIR(sctx->cur_inode_mode)) {
> >  		ret = did_create_dir(sctx, sctx->cur_ino);
> >  		if (ret < 0)
> > -			goto out;
> > -		if (ret) {
> > -			ret = 0;
> > -			goto out;
> > -		}
> > +			return ret;
> > +		if (ret > 0)
> > +			return 0;
> 
> nit: Personally I'd prefer in such cases to use an if/else if
> construct
> since which branch is taken is dependent on the same value. To me
> using
> an if/else if is a more explicit way to say "those 2 branches are
> directly related). However it's not a big deal.

Indeed, it's better. Should I send a v2 in this case or david can just
add an 'else' to the last if clause?

  Thanks

> 
> >  	}
> >  
> > -	ret = send_create_inode(sctx, sctx->cur_ino);
> > -	if (ret < 0)
> > -		goto out;
> > -
> > -out:
> > -	return ret;
> > +	return send_create_inode(sctx, sctx->cur_ino);
> >  }
> >  
> >  struct recorded_ref {
> >