xfs
[Top] [All Lists]

Re: [PATCH 13/27] xfs: factor out xfs_dir2_leaf_find_entry

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 13/27] xfs: factor out xfs_dir2_leaf_find_entry
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 30 Jun 2011 03:34:59 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20110630061102.GG561@dastard>
References: <20110629140109.003209430@xxxxxxxxxxxxxxxxxxxxxx> <20110629140339.086201354@xxxxxxxxxxxxxxxxxxxxxx> <20110630061102.GG561@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jun 30, 2011 at 04:11:02PM +1000, Dave Chinner wrote:
> You could probably just return here, and that would remove the:

Indeed.  We can do the same near the end, too.  It means duplicating
the be16_add_cpu, but allows to directly return the expression that
lead to lep, so I did it.

> > +                           continue;
> > +           }
> > +           /*
> > +            * If the low one is better, use it.
> > +            */
> 
> Line of whitespace before the comment.

Fixed.

> > +           /*
> > +            * The high one is better, so use that one.
> > +            */
> > +           } else {
> 
> I prefer comments inside the else branch...

The else is completely gone now, fixing that issue.

> > -           be16_add_cpu(&leaf->hdr.stale, -1);
> > -   }
> > +
> > +
> > +   lep = xfs_dir2_leaf_find_entry(leaf, index, compact, lowstale,
> > +                                  highstale, &lfloglow, &lfloghigh);
> > +
> 
> Only need one line of whitespace before the function call.

Fixed.

> > +
> > +
> >     /*
> >      * Insert the new entry, log everything.
> >      */
> > +   lep = xfs_dir2_leaf_find_entry(leaf, index, compact, lowstale,
> > +                                  highstale, &lfloglow, &lfloghigh);
> > +
> 
> Same for the whitespace before the comment.

Fixed.

<Prev in Thread] Current Thread [Next in Thread>