Re: [PATCH 16/19] fusermount: Refactor extract_x_options

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Mar 23, 2026 at 06:45:11PM +0100, Bernd Schubert wrote:
> From: Bernd Schubert <bschubert@ddn.com>
> 
> Just to make it better readable.
> 
> Also add a NULL check for *regular_opts and *x_prefixed_opts,
> there will be multiple callers in an upcoming commit and
> it becomes harder to track that no caller has an error.
> 
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>

Looks fine to me.  I would've added the NULL checks as a separate patch,
but this series is already plenty long...

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  util/fusermount.c | 58 ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/util/fusermount.c b/util/fusermount.c
> index ecf509bb80d5cd129f6e582f1ec666502c55603a..80b42a594e89cdc2f43824f5e274892522fd8cce 100644
> --- a/util/fusermount.c
> +++ b/util/fusermount.c
> @@ -112,22 +112,31 @@ static struct mntent *GETMNTENT(FILE *stream)
>  
>  /*
>   * Take a ',' separated option string and extract "x-" options
> + * @original: The original option string
> + * @regular_opts: The regular options
> + * @x_prefixed_opts: The "x-" options
>   */
> -static int extract_x_options(const char *original, char **non_x_opts,
> -			     char **x_opts)
> +static int extract_x_options(const char *original, char **regular_opts,
> +			     char **x_prefixed_opts)
>  {
>  	size_t orig_len;
>  	const char *opt, *opt_end;
>  
>  	orig_len = strlen(original) + 1;
>  
> -	*non_x_opts = calloc(1, orig_len);
> -	*x_opts    = calloc(1, orig_len);
> +	if (*regular_opts != NULL || *x_prefixed_opts != NULL) {
> +		fprintf(stderr, "%s: regular_opts or x_prefixed_opts not NULL\n",
> +			__func__);
> +		return -EINVAL;
> +	}
>  
> -	size_t non_x_opts_len = orig_len;
> -	size_t x_opts_len = orig_len;
> +	*regular_opts = calloc(1, orig_len);
> +	*x_prefixed_opts    = calloc(1, orig_len);
>  
> -	if (*non_x_opts == NULL || *x_opts == NULL) {
> +	size_t regular_opts_len = orig_len;
> +	size_t x_prefixed_opts_len = orig_len;
> +
> +	if (*regular_opts == NULL || *x_prefixed_opts == NULL) {
>  		fprintf(stderr, "%s: Failed to allocate %zuB.\n",
>  			__func__, orig_len);
>  		return -ENOMEM;
> @@ -143,16 +152,16 @@ static int extract_x_options(const char *original, char **non_x_opts,
>  		size_t opt_len = opt_end - opt;
>  		size_t opt_len_left = orig_len - (opt - original);
>  		size_t buf_len;
> -		bool is_x_opts;
> +		bool is_x_prefixed_opts;
>  
>  		if (strncmp(opt, "x-", MIN(2, opt_len_left)) == 0) {
> -			buf_len = x_opts_len;
> -			is_x_opts = true;
> -			opt_buf = *x_opts;
> +			buf_len = x_prefixed_opts_len;
> +			is_x_prefixed_opts = true;
> +			opt_buf = *x_prefixed_opts;
>  		} else {
> -			buf_len = non_x_opts_len;
> -			is_x_opts = false;
> -			opt_buf = *non_x_opts;
> +			buf_len = regular_opts_len;
> +			is_x_prefixed_opts = false;
> +			opt_buf = *regular_opts;
>  		}
>  
>  		if (buf_len < orig_len) {
> @@ -163,7 +172,8 @@ static int extract_x_options(const char *original, char **non_x_opts,
>  		/* omits ',' */
>  		if ((ssize_t)(buf_len - opt_len) < 0) {
>  			/* This would be a bug */
> -			fprintf(stderr, "%s: no buf space left in copy, orig='%s'\n",
> +			fprintf(stderr,
> +				"%s: no buf space left in copy, orig='%s'\n",
>  				__func__, original);
>  			return -EIO;
>  		}
> @@ -171,10 +181,10 @@ static int extract_x_options(const char *original, char **non_x_opts,
>  		strncat(opt_buf, opt, opt_end - opt);
>  		buf_len -= opt_len;
>  
> -		if (is_x_opts)
> -			x_opts_len = buf_len;
> +		if (is_x_prefixed_opts)
> +			x_prefixed_opts_len = buf_len;
>  		else
> -			non_x_opts_len = buf_len;
> +			regular_opts_len = buf_len;
>  	}
>  
>  	return 0;
> @@ -1379,7 +1389,7 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type)
>  	const char *real_mnt = mnt;
>  	int mountpoint_fd = -1;
>  	char *do_mount_opts = NULL;
> -	char *x_opts = NULL;
> +	char *x_prefixed_opts = NULL;
>  
>  	fd = open_fuse_device(dev);
>  	if (fd == -1)
> @@ -1397,7 +1407,7 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type)
>  	}
>  
>  	// Extract any options starting with "x-"
> -	res= extract_x_options(opts, &do_mount_opts, &x_opts);
> +	res = extract_x_options(opts, &do_mount_opts, &x_prefixed_opts);
>  	if (res)
>  		goto fail_close_fd;
>  
> @@ -1420,14 +1430,14 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type)
>  	}
>  
>  	if (geteuid() == 0) {
> -		if (x_opts && strlen(x_opts) > 0) {
> +		if (x_prefixed_opts && strlen(x_prefixed_opts) > 0) {
>  			/*
>  			 * Add back the options starting with "x-" to opts from
>  			 * do_mount. +2 for ',' and '\0'
>  			 */
>  			size_t mnt_opts_len = strlen(mnt_opts);
>  			size_t x_mnt_opts_len =  mnt_opts_len+
> -						 strlen(x_opts) + 2;
> +						 strlen(x_prefixed_opts) + 2;
>  			char *x_mnt_opts = calloc(1, x_mnt_opts_len);
>  
>  			if (mnt_opts_len) {
> @@ -1435,7 +1445,7 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type)
>  				strncat(x_mnt_opts, ",", 2);
>  			}
>  
> -			strncat(x_mnt_opts, x_opts,
> +			strncat(x_mnt_opts, x_prefixed_opts,
>  				x_mnt_opts_len - mnt_opts_len - 2);
>  
>  			free(mnt_opts);
> @@ -1452,7 +1462,7 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type)
>  out_free:
>  	free(source);
>  	free(mnt_opts);
> -	free(x_opts);
> +	free(x_prefixed_opts);
>  	free(do_mount_opts);
>  
>  	return fd;
> 
> -- 
> 2.43.0
> 
>