Re: [dm-devel] [PATCH 21/35] multipathd: move parse_cmd() to uxlsnr.c

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Sep 10, 2021 at 01:41:06PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> parse_cmd() does more than the name says - it parses, executes
> handlers, and even provides reply strings for some cases. This doesn't
> work well with the state machine idea. Thus move it to uxlsnr.c,
> where later patches will move some functionality elsewhere.
> 
> No functional changes.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/cli.c    | 74 +++++----------------------------------------
>  multipathd/cli.h    |  5 ++-
>  multipathd/uxlsnr.c | 61 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 67 deletions(-)
> 
> diff --git a/multipathd/cli.c b/multipathd/cli.c
> index f8c1dbd..29d6a6e 100644
> --- a/multipathd/cli.c
> +++ b/multipathd/cli.c
> @@ -256,8 +256,7 @@ find_key (const char * str)
>   * ESRCH: command not found
>   * EINVAL: argument missing for command
>   */
> -static int
> -get_cmdvec (char * cmd, vector *v)
> +int get_cmdvec (char *cmd, vector *v)
>  {
>  	int i;
>  	int r = 0;
> @@ -322,7 +321,7 @@ out:
>  }
>  
>  static uint64_t
> -fingerprint(vector vec)
> +fingerprint(const struct _vector *vec)
>  {
>  	int i;
>  	uint64_t fp = 0;
> @@ -337,6 +336,11 @@ fingerprint(vector vec)
>  	return fp;
>  }
>  
> +struct handler *find_handler_for_cmdvec(const struct _vector *v)
> +{
> +	return find_handler(fingerprint(v));
> +}
> +
>  int
>  alloc_handlers (void)
>  {
> @@ -415,8 +419,7 @@ do_genhelp(struct strbuf *reply, const char *cmd, int error) {
>  }
>  
>  
> -static char *
> -genhelp_handler (const char *cmd, int error)
> +char *genhelp_handler(const char *cmd, int error)
>  {
>  	STRBUF_ON_STACK(reply);
>  
> @@ -425,67 +428,6 @@ genhelp_handler (const char *cmd, int error)
>  	return steal_strbuf_str(&reply);
>  }
>  
> -int
> -parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout )
> -{
> -	int r;
> -	struct handler * h;
> -	vector cmdvec = NULL;
> -	struct timespec tmo;
> -
> -	r = get_cmdvec(cmd, &cmdvec);
> -
> -	if (r) {
> -		*reply = genhelp_handler(cmd, r);
> -		if (*reply == NULL)
> -			return EINVAL;
> -		*len = strlen(*reply) + 1;
> -		return 0;
> -	}
> -
> -	h = find_handler(fingerprint(cmdvec));
> -
> -	if (!h || !h->fn) {
> -		free_keys(cmdvec);
> -		*reply = genhelp_handler(cmd, EINVAL);
> -		if (*reply == NULL)
> -			return EINVAL;
> -		*len = strlen(*reply) + 1;
> -		return 0;
> -	}
> -
> -	/*
> -	 * execute handler
> -	 */
> -	if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) {
> -		tmo.tv_sec += timeout;
> -	} else {
> -		tmo.tv_sec = 0;
> -	}
> -	if (h->locked) {
> -		int locked = 0;
> -		struct vectors * vecs = (struct vectors *)data;
> -
> -		pthread_cleanup_push(cleanup_lock, &vecs->lock);
> -		if (tmo.tv_sec) {
> -			r = timedlock(&vecs->lock, &tmo);
> -		} else {
> -			lock(&vecs->lock);
> -			r = 0;
> -		}
> -		if (r == 0) {
> -			locked = 1;
> -			pthread_testcancel();
> -			r = h->fn(cmdvec, reply, len, data);
> -		}
> -		pthread_cleanup_pop(locked);
> -	} else
> -		r = h->fn(cmdvec, reply, len, data);
> -	free_keys(cmdvec);
> -
> -	return r;
> -}
> -
>  char *
>  get_keyparam (vector v, uint64_t code)
>  {
> diff --git a/multipathd/cli.h b/multipathd/cli.h
> index dbb75be..eed606a 100644
> --- a/multipathd/cli.h
> +++ b/multipathd/cli.h
> @@ -138,7 +138,10 @@ int __set_handler_callback (uint64_t fp, cli_handler *fn, bool locked);
>  #define set_handler_callback(fp, fn) __set_handler_callback(fp, fn, true)
>  #define set_unlocked_handler_callback(fp, fn) __set_handler_callback(fp, fn, false)
>  
> -int parse_cmd (char * cmd, char ** reply, int * len, void *, int);
> +int get_cmdvec (char *cmd, vector *v);
> +struct handler *find_handler_for_cmdvec(const struct _vector *v);
> +char *genhelp_handler (const char *cmd, int error);
> +
>  int load_keys (void);
>  char * get_keyparam (vector v, uint64_t code);
>  void free_keys (vector vec);
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 622aac1..cbbcb2c 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -311,6 +311,67 @@ static void handle_inotify(int fd, struct watch_descriptors *wds)
>  		condlog(1, "Multipath configuration updated.\nReload multipathd for changes to take effect");
>  }
>  
> +static int parse_cmd (char *cmd, char **reply, int *len, void *data,
> +		      int timeout)
> +{
> +	int r;
> +	struct handler * h;
> +	vector cmdvec = NULL;
> +	struct timespec tmo;
> +
> +	r = get_cmdvec(cmd, &cmdvec);
> +
> +	if (r) {
> +		*reply = genhelp_handler(cmd, r);
> +		if (*reply == NULL)
> +			return EINVAL;
> +		*len = strlen(*reply) + 1;
> +		return 0;
> +	}
> +
> +	h = find_handler_for_cmdvec(cmdvec);
> +
> +	if (!h || !h->fn) {
> +		free_keys(cmdvec);
> +		*reply = genhelp_handler(cmd, EINVAL);
> +		if (*reply == NULL)
> +			return EINVAL;
> +		*len = strlen(*reply) + 1;
> +		return 0;
> +	}
> +
> +	/*
> +	 * execute handler
> +	 */
> +	if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) {
> +		tmo.tv_sec += timeout;
> +	} else {
> +		tmo.tv_sec = 0;
> +	}
> +	if (h->locked) {
> +		int locked = 0;
> +		struct vectors * vecs = (struct vectors *)data;
> +
> +		pthread_cleanup_push(cleanup_lock, &vecs->lock);
> +		if (tmo.tv_sec) {
> +			r = timedlock(&vecs->lock, &tmo);
> +		} else {
> +			lock(&vecs->lock);
> +			r = 0;
> +		}
> +		if (r == 0) {
> +			locked = 1;
> +			pthread_testcancel();
> +			r = h->fn(cmdvec, reply, len, data);
> +		}
> +		pthread_cleanup_pop(locked);
> +	} else
> +		r = h->fn(cmdvec, reply, len, data);
> +	free_keys(cmdvec);
> +
> +	return r;
> +}
> +
>  static int uxsock_trigger(char *str, char **reply, int *len,
>  			  bool is_root, void *trigger_data)
>  {
> -- 
> 2.33.0

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