Re: [PATCH 3/5] metadump: separate single block objects from multiblock objects

[Dave Chinner <david@fromorbit.com>]

On Mon, Feb 03, 2014 at 07:09:07AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 23, 2014 at 09:23:53PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > When trying to dump objects, we have to treat multi-block objects
> > differently to single block objects. Separate out the code paths for
> > single block vs multi-block objects so we can add a separate path
> > for multi-block objects.
> 
> Looks good, but two minor style nitpicks below.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> >  static int
> > +process_multi_fsb_objects(
> > +	xfs_dfiloff_t	o,
> > +	xfs_dfsbno_t	s,
> > +	xfs_dfilblks_t	c,
> > +	typnm_t		btype,
> > +	xfs_dfiloff_t	last)
> > +{
> > +	if (btype != TYP_DIR2) {
> > +		print_warning("bad type for multi-fsb object %d", btype);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return process_single_fsb_objects(o, s, c, btype, last);
> 
> I'd prefer a switch with a default statement for the unknown type here,
> as that leads to nicer extensibility.

Ok, I'll fix that up.

> > +		/* single filesystem block objects are trivial to handle */
> > +		if (btype != TYP_DIR2 || mp->m_dirblkfsbs == 1) {
> > +			error = process_single_fsb_objects(o, s, c, btype, last);
> > +			if (error)
> >  				return 0;
> > +			continue;
> >  		}
> > +
> > +		/* multi-extent directory blocks */
> > +		error = process_multi_fsb_objects(o, s, c, btype, last);
> > +		if (error)
> > +			return 0;
> 
> An if / else would look a little more obvious here, not that it really
> matters all that much.

Easy enough to do.

Thanks for the reviews, Christoph.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs