On 01/04 2014 04:52 AM, Brian Foster wrote:
> On 12/28/2013 06:20 AM, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@xxxxxxxxxx>
>>
>> In xfs_bulkstat_single(), xfs_bulkstat_one() and xfs_bulkstat() might
>> return different error if either call failed, we'd better return the
>> proper error in this case. Moreover, the function argument done is
>> useless in terms of xfs_ioc_bulkstat(), hence we can get rid of it.
>>
>> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>> ---
>> fs/xfs/xfs_ioctl.c | 3 +--
>> fs/xfs/xfs_itable.c | 59 ++++++++++++++++++++++++++---------------------------
>> fs/xfs/xfs_itable.h | 3 +--
>> 3 files changed, 31 insertions(+), 34 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>> index 33ad9a7..2fdd750 100644
>> --- a/fs/xfs/xfs_ioctl.c
>> +++ b/fs/xfs/xfs_ioctl.c
>> @@ -828,8 +828,7 @@ xfs_ioc_bulkstat(
>> error = xfs_inumbers(mp, &inlast, &count,
>> bulkreq.ubuffer, xfs_inumbers_fmt);
>> else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE)
>> - error = xfs_bulkstat_single(mp, &inlast,
>> - bulkreq.ubuffer, &done);
>> + error = xfs_bulkstat_single(mp, &inlast, bulkreq.ubuffer);
>> else /* XFS_IOC_FSBULKSTAT */
>> error = xfs_bulkstat(mp, &inlast, &count, xfs_bulkstat_one,
>> sizeof(xfs_bstat_t), bulkreq.ubuffer,
>> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
>> index b5bb7b6..692671c 100644
>> --- a/fs/xfs/xfs_itable.c
>> +++ b/fs/xfs/xfs_itable.c
>> @@ -509,51 +509,50 @@ xfs_bulkstat(
>> }
>>
>> /*
>> - * Return stat information in bulk (by-inode) for the filesystem.
>> - * Special case for non-sequential one inode bulkstat.
>> + * Return stat information in bulk (by-inode) for the filesystem. Special case
>> + * for non-sequential one inode bulkstat.
>> */
>> -int /* error status */
>> +int
>> xfs_bulkstat_single(
>> - xfs_mount_t *mp, /* mount point for filesystem */
>> + struct xfs_mount *mp, /* mount point for filesystem */
>> xfs_ino_t *lastinop, /* inode to return */
>> - char __user *buffer, /* buffer with inode stats */
>> - int *done) /* 1 if there are more stats to get */
>> + char __user *buffer) /* buffer with inode stats */
>> {
>> - int count; /* count value for bulkstat call */
>> - int error; /* return value */
>> - xfs_ino_t ino; /* filesystem inode number */
>> + xfs_ino_t ino = *lastinop;
>> int res; /* result from bs1 */
>> + int error;
>>
>> /*
>> - * note that requesting valid inode numbers which are not allocated
>> + * Note that requesting valid inode numbers which are not allocated
>> * to inodes will most likely cause xfs_imap_to_bp to generate warning
>> - * messages about bad magic numbers. This is ok. The fact that
>> - * the inode isn't actually an inode is handled by the
>> - * error check below. Done this way to make the usual case faster
>> - * at the expense of the error case.
>> + * messages about bad magic numbers. This is ok. The fact that the
>> + * inode isn't actually an inode is handled by the error check below.
>> + * Done this way to make the usual case faster at the expense of the
>> + * error case.
>> */
>> -
>> - ino = *lastinop;
>> - error = xfs_bulkstat_one(mp, ino, buffer, sizeof(xfs_bstat_t),
>> + error = xfs_bulkstat_one(mp, ino, buffer, sizeof(struct xfs_bstat),
>> NULL, &res);
>> if (error) {
>> + int count = 1;
>> + int done, error2;
>> +
>> /*
>> - * Special case way failed, do it the "long" way
>> - * to see if that works.
>> + * Special case way failed, do it the "long" way to see if
>> + * that works.
>> */
>> (*lastinop)--;
>> - count = 1;
>> - if (xfs_bulkstat(mp, lastinop, &count, xfs_bulkstat_one,
>> - sizeof(xfs_bstat_t), buffer, done))
>> - return error;
>> - if (count == 0 || (xfs_ino_t)*lastinop != ino)
>> - return error == EFSCORRUPTED ?
>> - XFS_ERROR(EINVAL) : error;
>> - else
>> - return 0;
>> + error2 = xfs_bulkstat(mp, lastinop, &count, xfs_bulkstat_one,
>> + sizeof(struct xfs_bstat), buffer, &done);
>> + if (error2)
>> + return error2;
>> +
>> + if (count == 0 || *lastinop != ino) {
>> + if (error == EFSCORRUPTED)
>> + error = XFS_ERROR(EINVAL);
>> + }
>> }
>> - *done = 0;
>> - return 0;
>> +
>> + return error;
>
> Here it looks like you return an error value if the initial
> xfs_bulkstat_one() fails and the fallback xfs_bulkstat() succeeds.
Good catch, thanks for for your review.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
