xfs
[Top] [All Lists]

Re: [PATCH 1/4] xfs: refactor xlog_recover_process_data()

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/4] xfs: refactor xlog_recover_process_data()
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 26 Aug 2014 14:55:09 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140826040921.GA9591@xxxxxxxxxxxxx>
References: <1409016101-9511-1-git-send-email-david@xxxxxxxxxxxxx> <1409016101-9511-2-git-send-email-david@xxxxxxxxxxxxx> <20140826040921.GA9591@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Aug 25, 2014 at 09:09:21PM -0700, Christoph Hellwig wrote:
> > @@ -3556,14 +3622,10 @@ xlog_recover_process_data(
> >     xfs_caddr_t             dp,
> >     int                     pass)
> >  {
> > +   struct xlog_op_header   *ohead;
> >     xfs_caddr_t             lp;
> >     int                     num_logops;
> >     int                     error;
> >  
> >     lp = dp + be32_to_cpu(rhead->h_len);
> >     num_logops = be32_to_cpu(rhead->h_num_logops);
> > @@ -3573,69 +3635,24 @@ xlog_recover_process_data(
> >             return -EIO;
> >  
> >     while ((dp < lp) && num_logops) {
> > +           ASSERT(dp + sizeof(struct xlog_op_header) <= lp);
> > +
> > +           ohead = (struct xlog_op_header *)dp;
> > +           dp += sizeof(*ohead);
> 
> Using sizeof type and sizeof variable for the same thing right next
> to each other seems weird.  Also why duplicate the addition instead
> of moving it below the assignment:

Oh, I missed converting the one in the ASSERT.

>               ohead = (struct xlog_op_header *)dp;
>               dp += sizeof(*ohead);
> 
>               ASSERT(dp <= lp);

Yup, that makes sense.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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