Re: [dm-devel] [PATCH v2 27/29] multipathd: common code for "-k" and command args

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Oct 16, 2020 at 12:44:59PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> 'multipathd -k"cmd"' and 'multipath cmd' are the same thing.
> Treat it with common code.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/main.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 725a10b..0cf8a26 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -3303,6 +3303,8 @@ main (int argc, char *argv[])
>  	int err;
>  	int foreground = 0;
>  	struct config *conf;
> +	char *opt_k_arg = NULL;
> +	bool opt_k = false;
>  
>  	ANNOTATE_BENIGN_RACE_SIZED(&multipath_conf, sizeof(multipath_conf),
>  				   "Manipulated through RCU");
> @@ -3350,16 +3352,9 @@ main (int argc, char *argv[])
>  			logsink = LOGSINK_STDERR_WITHOUT_TIME;
>  			break;
>  		case 'k':
> -			logsink = 0;
> -			conf = load_config(DEFAULT_CONFIGFILE);
> -			if (!conf)
> -				exit(1);
> -			if (verbosity)
> -				libmp_verbosity = verbosity;
> -			uxsock_timeout = conf->uxsock_timeout;
> -			err = uxclnt(optarg, uxsock_timeout + 100);
> -			free_config(conf);
> -			return err;
> +			opt_k = true;
> +			opt_k_arg = optarg;
> +			break;
>  		case 'B':
>  			bindings_read_only = 1;
>  			break;
> @@ -3375,7 +3370,7 @@ main (int argc, char *argv[])
>  			exit(1);
>  		}
>  	}
> -	if (optind < argc) {
> +	if (opt_k || optind < argc) {
>  		char cmd[CMDSIZE];
>  		char * s = cmd;
>  		char * c = s;
> @@ -3390,14 +3385,20 @@ main (int argc, char *argv[])
>  			libmp_verbosity = verbosity;
>  		uxsock_timeout = conf->uxsock_timeout;
>  		memset(cmd, 0x0, CMDSIZE);
> -		while (optind < argc) {
> -			if (strchr(argv[optind], ' '))
> -				c += snprintf(c, s + CMDSIZE - c, "\"%s\" ", argv[optind]);
> -			else
> -				c += snprintf(c, s + CMDSIZE - c, "%s ", argv[optind]);
> -			optind++;
> +		if (opt_k)
> +			s = opt_k_arg;
> +		else {
> +			while (optind < argc) {
> +				if (strchr(argv[optind], ' '))
> +					c += snprintf(c, s + CMDSIZE - c,
> +						      "\"%s\" ", argv[optind]);
> +				else
> +					c += snprintf(c, s + CMDSIZE - c,
> +						      "%s ", argv[optind]);
> +				optind++;
> +			}
> +			c += snprintf(c, s + CMDSIZE - c, "\n");

combining these two methods makes is obvious that adding the newline to
the cmd buffer is unnecessary. But since your patch didn't add it, and
it doesn't hurt anything
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

>  		}
> -		c += snprintf(c, s + CMDSIZE - c, "\n");
>  		err = uxclnt(s, uxsock_timeout + 100);
>  		free_config(conf);
>  		return err;
> -- 
> 2.28.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel