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

[Eric Sandeen <sandeen@xxxxxxxxxxx>]

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