Re: [PATCH] btrfs: reorganize error handling in btrfs_tree_mod_log_insert_key

[Sun YangKai <sunk67188@gmail.com>]

> > -       ret = tree_mod_log_insert(eb->fs_info, tm);
> > -out_unlock:
> > +       /* Deal with allocation error. */
> 
> This is a useless comment... The existing comment is much more helpful
> and the fact that's in the else statement above, makes it easier to
> grok.

I agree that the existing comment is much more helpful, while I personally 
don't like the else branch appears after a return/goto/continue/break 
statement. So I think it's just about personal code style.

> I also wonder why you picked only this function, since this pattern is
> followed in several other functions...

Because I happened to read this, and it takes me minutes to realise what is 
happening and why it was written like this...
And I'm not sure how to make it better. Since this is the most simple one with 
this pattern, I just have a try to make it more clear IMO.

> Not a fan of the proposed change.
> 
> > +       if (tm)
> > +               ret = tree_mod_log_insert(eb->fs_info, tm);
> > +       else
> > +               ret = -ENOMEM;
> > +
> > 
> >         write_unlock(&eb->fs_info->tree_mod_log_lock);
> >         if (ret)
> >         
> >                 kfree(tm);
> > 
> > --
> > 2.51.0