Re: REVIEW: xfs_bmap_check_leaf_extents() can reference unmapped memory

[Christoph Hellwig <hch@infradead.org>]

On Thu, Mar 20, 2008 at 05:11:23PM +1100, Lachlan McIlroy wrote:
> +	xfs_bmbt_rec_t		last;   /* last extent in previous block */
>  	xfs_bmbt_rec_t		*nextp;	/* pointer to next extent */
>  	int			bp_release = 0;
>
> @@ -6264,7 +6264,6 @@ xfs_bmap_check_leaf_extents(
>  	/*
>  	 * Loop over all leaf nodes checking that all extents are in the right order.
>  	 */
>  	for (;;) {
>  		xfs_fsblock_t	nextbno;
>  		xfs_extnum_t	num_recs;
> @@ -6285,18 +6284,18 @@ xfs_bmap_check_leaf_extents(
>  		 */
>
>  		ep = XFS_BTREE_REC_ADDR(xfs_bmbt, block, 1);
> +		if (i) {
> +			xfs_btree_check_rec(XFS_BTNUM_BMAP, (void *)&last,
> +				(void *)ep);

I haven't actually compiled this yet, but I'd expect this to give an
unitialized variable warning with gcc because it can't figure out this
can't happen in the first loop iteration.  You might need and
last = { 0, } somewhere in the beginning of the function.

Also I think the void * casts above are useless.

>  			xfs_btree_check_rec(XFS_BTNUM_BMAP, (void *)ep,
>  				(void *)(nextp));

and at that point you might fix these up aswell, with the added benefit
that now the whole call fits on a single line.