Re: [PATCH v2] Make --mount-point option generally available

[Yue Hu <zbestahu@gmail.com>]

Hi Kelvin,

On Fri,  1 Jul 2022 16:00:30 -0700
Kelvin Zhang via Linux-erofs <linux-erofs@lists.ozlabs.org> wrote:

> This option does not have any android specific dependencies. It is also
> useful for all selinux enabled fs images, so move it out of android
> specific feature sets.
> 
> e.g. mkfs.erofs --file-contexts=selinux_context_file
> --mount_point=/product product.img your_product_out_dir
> 
> Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
> ---
>  include/erofs/config.h | 2 +-
>  lib/xattr.c            | 2 --
>  mkfs/main.c            | 6 +++---
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/include/erofs/config.h b/include/erofs/config.h
> index 0a1b18b..030054b 100644
> --- a/include/erofs/config.h
> +++ b/include/erofs/config.h
> @@ -65,8 +65,8 @@ struct erofs_configure {
>  	u32 c_dict_size;
>  	u64 c_unix_timestamp;
>  	u32 c_uid, c_gid;
> +	const char *mount_point;
>  #ifdef WITH_ANDROID
> -	char *mount_point;
>  	char *target_out_path;
>  	char *fs_config_file;
>  	char *block_list_file;
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 00fb963..cf5c447 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -210,12 +210,10 @@ static struct xattr_item *erofs_get_selabel_xattr(const char *srcpath,
>  		unsigned int len[2];
>  		char *kvbuf, *fspath;
>  
> -#ifdef WITH_ANDROID
>  		if (cfg.mount_point)
>  			ret = asprintf(&fspath, "/%s/%s", cfg.mount_point,
>  				       erofs_fspath(srcpath));
>  		else
> -#endif
>  			ret = asprintf(&fspath, "/%s", erofs_fspath(srcpath));
>  		if (ret <= 0)
>  			return ERR_PTR(-ENOMEM);
> diff --git a/mkfs/main.c b/mkfs/main.c
> index b62a8aa..879c2f2 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -50,8 +50,8 @@ static struct option long_options[] = {
>  	{"quiet", no_argument, 0, 12},
>  	{"blobdev", required_argument, NULL, 13},
>  	{"ignore-mtime", no_argument, NULL, 14},
> -#ifdef WITH_ANDROID
>  	{"mount-point", required_argument, NULL, 512},
> +#ifdef WITH_ANDROID
>  	{"product-out", required_argument, NULL, 513},
>  	{"fs-config-file", required_argument, NULL, 514},
>  	{"block-list-file", required_argument, NULL, 515},
> @@ -103,9 +103,9 @@ static void usage(void)
>  #ifndef NDEBUG
>  	      " --random-pclusterblks randomize pclusterblks for big pcluster (debugging only)\n"
>  #endif
> +	      " --mount-point=X       X=prefix of target fs path (default: /)\n"
>  #ifdef WITH_ANDROID
>  	      "\nwith following android-specific options:\n"
> -	      " --mount-point=X       X=prefix of target fs path (default: /)\n"
>  	      " --product-out=X       X=product_out directory\n"
>  	      " --fs-config-file=X    X=fs_config file\n"
>  	      " --block-list-file=X   X=block_list file\n"
> @@ -314,7 +314,6 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
>  		case 10:
>  			cfg.c_compress_hints_file = optarg;
>  			break;
> -#ifdef WITH_ANDROID
>  		case 512:
>  			cfg.mount_point = optarg;
>  			/* all trailing '/' should be deleted */
> @@ -322,6 +321,7 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
>  			if (opt && optarg[opt - 1] == '/')
>  				optarg[opt - 1] = '\0';
>  			break;
> +#ifdef WITH_ANDROID
>  		case 513:
>  			cfg.target_out_path = optarg;
>  			break;

As Xiang pointed out, it is common convention to prefix the subject line to let us
distinguish from others more easily.