linux-kernel - Re: [PATCH] nilfs2: delete unnecessary checks before brelse()

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 20 Aug 2022 08:50:59 +0900
From:   Ryusuke Konishi <konishi.ryusuke@...il.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     "cgel.zte@...il.com" <cgel.zte@...il.com>,
        "linux-nilfs@...r.kernel.org" <linux-nilfs@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Minghao Chi <chi.minghao@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH] nilfs2: delete unnecessary checks before brelse()

Hi David,

On Sat, Aug 20, 2022 at 6:31 AM David Laight wrote:
>
> From: cgel.zte@...il.com
> > Sent: 19 August 2022 09:17
> >
> > The brelse() function tests whether its argument is NULL
> > and then returns immediately.
> > Thus remove the tests which are not needed around the shown calls.
>

> The test is the caller makes sense if it is a 'hot' path
> and bh is normally NULL.
>
>         David

Should the commit message say "inline function" instead of "function"?

or do you have other intentions?

These tests indeed seem meaningless, since brelse() is an inline
function that tests whether bh is NULL or not, like this

 static inline void brelse(struct buffer_head *bh)
 {
        if (bh)
                __brelse(bh);
 }

and the corresponding parts are equivalent to:

  if (bh != NULL)
          if (bh)
                  __brelse(bh);


Regards,
Ryusuke Konishi

>
> >
> > Reported-by: Zeal Robot <zealci@....com.cn>
> > Signed-off-by: Minghao Chi <chi.minghao@....com.cn>
> > ---
> >  fs/nilfs2/btree.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
> > index f544c22fff78..b3b5a46488c2 100644
> > --- a/fs/nilfs2/btree.c
> > +++ b/fs/nilfs2/btree.c
> > @@ -1668,8 +1668,7 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
> >       maxkey = nilfs_btree_node_get_key(node, nchildren - 1);
> >       nextmaxkey = (nchildren > 1) ?
> >               nilfs_btree_node_get_key(node, nchildren - 2) : 0;
> > -     if (bh != NULL)
> > -             brelse(bh);
> > +     brelse(bh);
> >
> >       return (maxkey == key) && (nextmaxkey < NILFS_BMAP_LARGE_LOW);
> >  }
> > @@ -1717,8 +1716,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *btree,
> >               ptrs[i] = le64_to_cpu(dptrs[i]);
> >       }
> >
> > -     if (bh != NULL)
> > -             brelse(bh);
> > +     brelse(bh);
> >
> >       return nitems;
> >  }
> > --
> > 2.25.1