Re: [LTP] [PATCH v3 4/4] tst_find_backing_dev: Also check /dev/block/ for backing device

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
>  -------------------------------------------------------------------------------
>  #include "tst_test.h"
>  
> -void tst_find_backing_dev(const char *path, char *dev);
> +void tst_find_backing_dev(const char *path, char *dev, size_t dev_len);
>  -------------------------------------------------------------------------------
>  
>  This function finds the block dev that this path belongs to, using uevent in sysfs.
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 977427f1c..f0aff4473 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -110,8 +110,9 @@ void tst_purge_dir(const char *path);
>   * Find the file or path belongs to which block dev
>   * @path  Path to find the backing dev
>   * @dev   The block dev
> + * @dev_len   The length of the dev string
>   */
> -void tst_find_backing_dev(const char *path, char *dev);
> +void tst_find_backing_dev(const char *path, char *dev, size_t dev_len);
                                                                  ^
This would be a bit better as dev_size since the common usage is that
strings have length and buffers have size.

>  /*
>   * Stat the device mounted on a given path.
> diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
> index 87cec3961..53099f9bc 100644
> --- a/lib/newlib_tests/tst_device.c
> +++ b/lib/newlib_tests/tst_device.c
> @@ -71,7 +71,7 @@ static void test_tst_find_backing_dev(void)
>  {
>  	char block_dev[100];
>  
> -	tst_find_backing_dev(mntpoint, block_dev);
> +	tst_find_backing_dev(mntpoint, block_dev, sizeof(block_dev));
>  
>  	if (!strcmp(tst_device->dev, block_dev))
>  		tst_res(TPASS, "%s belongs to %s block dev", mntpoint,
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index ba46b7613..74de307e7 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -60,6 +60,11 @@ static const char *dev_loop_variants[] = {
>  	"/dev/block/loop%i"
>  };
>  
> +static const char *dev_variants[] = {
> +	"/dev/%s",
> +	"/dev/block/%s"
> +};
> +
>  static int set_dev_loop_path(int dev, char *path, size_t path_len)
>  {
>  	unsigned int i;
> @@ -75,6 +80,21 @@ static int set_dev_loop_path(int dev, char *path, size_t path_len)
>  	return 1;
>  }
>  
> +static int set_dev_path(char *dev, char *path, size_t path_len)
> +{
> +	unsigned int i;
> +	struct stat st;
> +
> +	for (i = 0; i < ARRAY_SIZE(dev_variants); i++) {
> +		snprintf(path, path_len, dev_variants[i], dev);
> +
> +		if (stat(path, &st) == 0 && S_ISBLK(st.st_mode))
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
>  int tst_find_free_loopdev(char *path, size_t path_len)
>  {
>  	int ctl_fd, dev_fd, rc, i;
> @@ -511,7 +531,7 @@ unsigned long tst_dev_bytes_written(const char *dev)
>  }
>  
>  __attribute__((nonnull))
> -void tst_find_backing_dev(const char *path, char *dev)
> +void tst_find_backing_dev(const char *path, char *dev, size_t dev_len)
>  {
>  	struct stat buf;
>  	struct btrfs_ioctl_fs_info_args args = {0};
> @@ -574,7 +594,7 @@ void tst_find_backing_dev(const char *path, char *dev)
>  			sprintf(uevent_path, "%s/%s/uevent",
>  				bdev_path, d->d_name);
>  		} else {
> -			tst_brkm(TBROK | TERRNO, NULL, "No backining device found while looking in %s.", bdev_path);
> +			tst_brkm(TBROK | TERRNO, NULL, "No backing device found while looking in %s.", bdev_path);
>  		}
>  
>  		if (SAFE_READDIR(NULL, dir))
> @@ -590,17 +610,12 @@ void tst_find_backing_dev(const char *path, char *dev)
>  	if (!access(uevent_path, R_OK)) {
>  		FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s", dev_name);
>  
> -		if (dev_name[0])
> -			sprintf(dev, "/dev/%s", dev_name);
> +		if (!dev_name[0] || set_dev_path(dev_name, dev, dev_len) != 0)
                                                                          ^
									  isn't
									  this
									  redundant?

These two are the only minor issues I've found in the patchset,
otherwise:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp