xfs
[Top] [All Lists]

Re: [PATCH v2 05/10] xfs: fix error handling in xfs_bulkstat

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH v2 05/10] xfs: fix error handling in xfs_bulkstat
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Fri, 25 Apr 2014 15:21:41 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140425064815.GB20871@xxxxxxxxxxxxx>
References: <535078B8.4020905@xxxxxxxxxx> <20140425064815.GB20871@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
On 04/25 2014 14:48 PM, Christoph Hellwig wrote:
>> Moreover, this fix also get rid of the redundant user buffer count
>> pre-checkups as it has already been validated in upper callers.
> 
>> -    if (!ubcountp || *ubcountp <= 0) {
>> -            return EINVAL;
>> -    }
> 
> Probably better to have this as a separate patch.

Sometimes, I'd to put such kind of trivial fixes into a relative effective patch
if possible. But from another point of view, yep, have it as a separate patch 
would
make it more convenient for reviewer.

> 
>> -                    /*
>> -                     * Loop as long as we're unable to read the
>> -                     * inode btree.
>> -                     */
>> -                    while (error) {
>> -                            agino += XFS_INODES_PER_CHUNK;
>> -                            if (XFS_AGINO_TO_AGBNO(mp, agino) >=
>> -                                            be32_to_cpu(agi->agi_length))
>> -                                    break;
>> -                            error = xfs_inobt_lookup(cur, agino,
>> -                                                     XFS_LOOKUP_GE, &tmp);
>> -                            cond_resched();
>> -                    }
> 
> This code goes back to 1995, but I still can't see how it would make
> sense.  I think we should get rid of this, but I'd also love to have
> Dave and Eric double check it as well.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Thanks,
-Jeff

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