Re: [PATCH] XFS: possibly uninitialized variable use in fs/xfs/xfs_da_btree.c::xfs_da_node_lookup_int()

Eric Sandeen
2006-08-13T03:25:55Z

Jesper Juhl wrote:
> I suspect I may be completely wrong, and if that's the case I'd sure like
> an explanation of where I went wrong along with the NACK for the patch.
> In case my understanding is in fact correct and the patch below makes sense,
> then kindly apply :-)
>

Seems reasonable to me; I also can't see how it avoids using an uninitialized
retval in all cases. But I'm in over my head too, so don't take my word for it
:). I'll let the folks @sgi chime in...

FWIW seems like there's a lot of unnecessary endian flipping in there too; I
haven't tested this but since it endian-flips the magic into blk->magic seems
like it may as well use it:

Index: xfs-linux/xfs_da_btree.c
===================================================================
--- xfs-linux.orig/xfs_da_btree.c
+++ xfs-linux/xfs_da_btree.c
@@ -1079,14 +1079,14 @@ xfs_da_node_lookup_int(xfs_da_state_t *s
return(error);
}
curr = blk->bp->data;
- ASSERT(be16_to_cpu(curr->magic) == XFS_DA_NODE_MAGIC ||
- be16_to_cpu(curr->magic) == XFS_DIR2_LEAFN_MAGIC ||
- be16_to_cpu(curr->magic) == XFS_ATTR_LEAF_MAGIC);
+ blk->magic = be16_to_cpu(curr->magic);
+ ASSERT(blk->magic == XFS_DA_NODE_MAGIC ||
+ blk->magic == XFS_DIR2_LEAFN_MAGIC ||
+ blk->magic == XFS_ATTR_LEAF_MAGIC);

/*
* Search an intermediate node for a match.
*/
- blk->magic = be16_to_cpu(curr->magic);
if (blk->magic == XFS_DA_NODE_MAGIC) {
node = blk->bp->data;
blk->hashval =
be32_to_cpu(node->btree[be16_to_cpu(node->hdr.count)-1].hashval);
@@ -1133,10 +1133,10 @@ xfs_da_node_lookup_int(xfs_da_state_t *s
blk->index = probe;
blkno = be32_to_cpu(btree->before);
}
- } else if (be16_to_cpu(curr->magic) == XFS_ATTR_LEAF_MAGIC) {
+ } else if (blk->magic == XFS_ATTR_LEAF_MAGIC) {
blk->hashval = xfs_attr_leaf_lasthash(blk->bp, NULL);
break;
- } else if (be16_to_cpu(curr->magic) == XFS_DIR2_LEAFN_MAGIC) {
+ } else if (blk->magic == XFS_DIR2_LEAFN_MAGIC) {
blk->hashval = xfs_dir2_leafn_lasthash(blk->bp, NULL);
break;
}