2022-07-06 14:16:42

by Andrey Strachuk

Subject: [PATCH] xfs: removed useless condition in function xfs_attr_node_get

At line 1561, variable "state" is being compared
with NULL every loop iteration.

-------------------------------------------------------------------
1561 for (i = 0; state != NULL && i < state->path.active; i++) {
1562 xfs_trans_brelse(args->trans, state->path.blk[i].bp);
1563 state->path.blk[i].bp = NULL;
1564 }
-------------------------------------------------------------------

However, it cannot be NULL.

----------------------------------------
1546 state = xfs_da_state_alloc(args);
----------------------------------------

xfs_da_state_alloc calls kmem_cache_zalloc. kmem_cache_zalloc is
called with __GFP_NOFAIL flag and, therefore, it cannot return NULL.

--------------------------------------------------------------------------
struct xfs_da_state *
xfs_da_state_alloc(
struct xfs_da_args *args)
{
struct xfs_da_state *state;

state = kmem_cache_zalloc(xfs_da_state_cache, GFP_NOFS | __GFP_NOFAIL);
state->args = args;
state->mp = args->dp->i_mount;
return state;
}
--------------------------------------------------------------------------

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Andrey Strachuk <[email protected]>

Fixes: 4d0cdd2bb8f0 ("xfs: clean up xfs_attr_node_hasname")
---
fs/xfs/libxfs/xfs_attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 224649a76cbb..6b8857e53add 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1558,7 +1558,7 @@ xfs_attr_node_get(
* If not in a transaction, we have to release all the buffers.
*/
out_release:
- for (i = 0; state != NULL && i < state->path.active; i++) {
+ for (i = 0; i < state->path.active; i++) {
xfs_trans_brelse(args->trans, state->path.blk[i].bp);
state->path.blk[i].bp = NULL;
}
--
2.25.1

2022-07-06 17:23:48

by Darrick J. Wong

Subject: Re: [PATCH] xfs: removed useless condition in function xfs_attr_node_get

On Wed, Jul 06, 2022 at 04:36:27PM +0300, Andrey Strachuk wrote:
> At line 1561, variable "state" is being compared
> with NULL every loop iteration.
>
> -------------------------------------------------------------------
> 1561 for (i = 0; state != NULL && i < state->path.active; i++) {
> 1562 xfs_trans_brelse(args->trans, state->path.blk[i].bp);
> 1563 state->path.blk[i].bp = NULL;
> 1564 }
> -------------------------------------------------------------------
>
> However, it cannot be NULL.
>
> ----------------------------------------
> 1546 state = xfs_da_state_alloc(args);
> ----------------------------------------
>
> xfs_da_state_alloc calls kmem_cache_zalloc. kmem_cache_zalloc is
> called with __GFP_NOFAIL flag and, therefore, it cannot return NULL.
>
> --------------------------------------------------------------------------
> struct xfs_da_state *
> xfs_da_state_alloc(
> struct xfs_da_args *args)
> {
> struct xfs_da_state *state;
>
> state = kmem_cache_zalloc(xfs_da_state_cache, GFP_NOFS | __GFP_NOFAIL);
> state->args = args;
> state->mp = args->dp->i_mount;
> return state;
> }
> --------------------------------------------------------------------------
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Andrey Strachuk <[email protected]>
>
> Fixes: 4d0cdd2bb8f0 ("xfs: clean up xfs_attr_node_hasname")

Nit: usually the tags go at the end with no empty lines between them.

The change looks correct to me, so
Reviewed-by: Darrick J. Wong <[email protected]>

--D

> ---
> fs/xfs/libxfs/xfs_attr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 224649a76cbb..6b8857e53add 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1558,7 +1558,7 @@ xfs_attr_node_get(
> * If not in a transaction, we have to release all the buffers.
> */
> out_release:
> - for (i = 0; state != NULL && i < state->path.active; i++) {
> + for (i = 0; i < state->path.active; i++) {
> xfs_trans_brelse(args->trans, state->path.blk[i].bp);
> state->path.blk[i].bp = NULL;
> }
> --
> 2.25.1
>