Re: [PATCH] xfsprogs: fix cut & paste error in xfs_fsr.c

[Eric Sandeen <sandeen@redhat.com>]

> On Tue, Nov 10, 2015 at 12:21:10PM -0600, Eric Sandeen wrote:
>> Commit:
>>
>> 7141fc xfsprogs: make fsr use mntinfo when there is no mntent
>>
>> added an inadvertent "break;" to initallfs() after the call
>> to find_mountpoint_check(); this is likely a cut & paste error
>> from the call in find_mountpoint(), where we really *do* want to
>> stop after the first one we find.
>>
>> Fix that by removing the break, and fix the declaration-after-code.
> 
> That's not the right fix - the find_mountpoint_check() shoul dnot be
> there at all. Patch to clean it all up is below.
> 
> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 
> fsr: clean up mountpoint checks
> 
> From: Dave Chinner <dchinner@redhat.com>
> 
> Fix up a stray hunk of code from commit 7141fc5 ("xfsprogs: make fsr
> use mntinfo when there is no mntent") that coverity reported. Also
> clean up a couple of whitespace issues introduced with that  commit,
> too.

Um, ok, well it wasn't your misapplication, it was there in the original patch,
and I wasn't sure of Jan's intent, having gone so far as to add local vars
to support that placement.  ;)

This seems fine, I guess I'll send another patch to make *ms local to 
find_mountpoint_check(), and rename some vars ("sb" and "mp" have their
own expected meanings in xfsprogs)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

-Eric

> Addresses-Coverity-Id: 1338431
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fsr/xfs_fsr.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index b902acc..2887ceb 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -176,7 +176,6 @@ aborter(int unused)
>   * here - the code that handles defragmentation of invidual files takes care
>   * of that.
>   */
> -
>  static char *
>  find_mountpoint_check(struct stat64 *sb, struct mntent *t, struct stat64 *ms)
>  {
> @@ -200,9 +199,9 @@ find_mountpoint_check(struct stat64 *sb, struct mntent *t, struct stat64 *ms)
>  			return NULL;
>  
>  		/*
> -			* Make sure the mountpoint given by mtab is accessible
> -			* before using it.
> -			*/
> +		 * Make sure the mountpoint given by mtab is accessible
> +		 * before using it.
> +		 */
>  		if (stat64(t->mnt_dir, &sb2) < 0)
>  			return NULL;
>  	}
> @@ -224,7 +223,7 @@ find_mountpoint(char *mtab, char *argname, struct stat64 *sb)
>  		exit(1);
>  	}
>  
> -	while ( (t = platform_mntent_next(&cursor)) != NULL) {
> +	while ((t = platform_mntent_next(&cursor)) != NULL) {
>  		mntp = find_mountpoint_check(sb, t, &ms);
>  		if (mntp == NULL)
>  			continue;
> @@ -409,12 +408,10 @@ static void
>  initallfs(char *mtab)
>  {
>  	struct mntent_cursor cursor;
> -	char *mntp = NULL;
>  	struct mntent *mp = NULL;
>  	int mi;
>  	char *cp;
>  	struct stat64 sb;
> -	struct stat64 ms;
>  
>  	/* malloc a number of descriptors, increased later if needed */
>  	if (!(fsbase = (fsdesc_t *)malloc(fsbufsize * sizeof(fsdesc_t)))) {
> @@ -432,12 +429,7 @@ initallfs(char *mtab)
>  		exit(1);
>  	}
>  
> -	while ( (mp = platform_mntent_next(&cursor)) != NULL) {
> -		mntp = find_mountpoint_check(&sb, mp, &ms);
> -		if (mntp == NULL)
> -			continue;
> -		break;
> -
> +	while ((mp = platform_mntent_next(&cursor)) != NULL) {
>  		int rw = 0;
>  
>  		if (strcmp(mp->mnt_type, MNTTYPE_XFS ) != 0 ||
> 

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