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

[Eric Sandeen <sandeen@sandeen.net>]

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;
  		}