Re: [PATCH 2/6] libxfs/linux.c: Replace use of ustat by stat

[Dave Chinner <david@fromorbit.com>]

On Tue, Jan 12, 2016 at 08:59:45PM +0100, Felix Janda wrote:
> ustat has been used to check whether a device file is mounted.
> The function is deprecated and not supported by uclibc and musl.
> Now do the check using the *mntent functions.
> 
> Based on patch by Natanael Copa <ncopa@alpinelinux.org>.
> 
> Signed-off-by: Felix Janda <felix.janda@posteo.de>

So, nobody has bothered to actually test this patch...

$ sudo mkfs.xfs -f -m rmapbt=1 -i sparse=1 -b size=1k /dev/sdc
specified blocksize 1024 is less than device physical sector size 4096
switching to logical sector size 512
Usage: mkfs.xfs
/* blocksize */         [-b log=n|size=num]
/* metadata */          [-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1]
....
$ sudo xfs_db /dev/sdc

fatal error -- couldn't initialize XFS library
$

And so on. Basic sanity testing fails.

> @@ -63,14 +61,27 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
>  		s = &st;
>  	}
>  
> -	if (ustat(s->st_rdev, ust) >= 0) {
> +	strcpy(mounts, (!access(PROC_MOUNTED, R_OK)) ? PROC_MOUNTED : MOUNTED);
> +	if ((f = setmntent(mounts, "r")) == NULL) {
> +		fprintf(stderr,
> +		    _("%s: %s possibly contains a mounted filesystem\n"),
> +		    progname, name);
> +		return 1;
> +	}
> +	while ((mnt = getmntent(f)) != NULL) {
> +		if (stat64(mnt->mnt_dir, &mst) < 0)
> +			continue;
> +		if (mst.st_dev != s->st_rdev)
> +			continue;
> +
>  		if (verbose)
>  			fprintf(stderr,
>  				_("%s: %s contains a mounted filesystem\n"),
>  				progname, name);
> -		return 1;
> +		break;
>  	}
> -	return 0;
> +	endmntent(f);
> +	return mnt == NULL;

Because that return statement has the wrong logic. It should be:

+	return mnt != NULL;

to return the correct values.

In future, please do more than a compile test on your patches before
sending them to the list for review, no matter how simple and
obviously correct you think they are!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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