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@xxxxxxxxxx>
> >
> > 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@xxxxxx>
>
> > 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@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
