All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Liu <jeff.liu@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: "xfs@oss.sgi.com" <xfs@oss.sgi.com>
Subject: Re: [PATCH v2 05/10] xfs: fix error handling in xfs_bulkstat
Date: Fri, 25 Apr 2014 15:21:41 +0800	[thread overview]
Message-ID: <535A0D05.1090004@oracle.com> (raw)
In-Reply-To: <20140425064815.GB20871@infradead.org>


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@lst.de>

Thanks,
-Jeff

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

  reply	other threads:[~2014-04-25  7:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-18  0:58 [PATCH v2 05/10] xfs: fix error handling in xfs_bulkstat Jeff Liu
2014-04-25  6:48 ` Christoph Hellwig
2014-04-25  7:21   ` Jeff Liu [this message]
2014-04-27 21:26   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=535A0D05.1090004@oracle.com \
    --to=jeff.liu@oracle.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.