Re: [PATCH v2 1/2] btrfs: fix silent failure when deleting root reference

[David Sterba <dsterba@suse.cz>]

On Tue, Aug 23, 2022 at 09:11:55AM +0100, Filipe Manana wrote:
> On Tue, Aug 23, 2022 at 12:47 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> > > ---
> > >   fs/btrfs/root-tree.c | 5 +++--
> > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> > > index a64b26b16904..d647cb2938c0 100644
> > > --- a/fs/btrfs/root-tree.c
> > > +++ b/fs/btrfs/root-tree.c
> > > @@ -349,9 +349,10 @@ int btrfs_del_root_ref(struct btrfs_trans_handle *trans, u64 root_id,
> > >       key.offset = ref_id;
> > >   again:
> > >       ret = btrfs_search_slot(trans, tree_root, &key, path, -1, 1);
> > > -     if (ret < 0)
> > > +     if (ret < 0) {
> > > +             err = ret;
> > >               goto out;
> > > -     if (ret == 0) {
> >
> > Just a small nitpick here, since above if (ret < 0) branch will call
> > "goto out", we don't need the "else" branch.
> > The old "if (ret == 0) {" should be good enough.
> 
> Yes, it's not about correctness. It's just my preferred style.
> I find it more intuitive to have a single if-else that tests the same
> variable instead
> of having it tested by two distinct if statements.

Agreed with that style.