[dm-devel] [PATCH 2/5] libmultipath: don't copy pgpolicy string in get_pgpolicy_name

[Martin Wilck]

On Fri, 2023-05-19 at 18:02 -0500, Benjamin Marzinski wrote:
> copying the value into a passed in buffer doesn't help any of the
> callers of this function. It's just wasted work.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>

Reviewed-by: Martin Wilck <mwilck at suse.com>

> ---
>  libmultipath/dict.c       |  6 +-----
>  libmultipath/pgpolicies.c | 27 ++++++++-------------------
>  libmultipath/pgpolicies.h |  2 +-
>  libmultipath/propsel.c    |  6 ++----
>  4 files changed, 12 insertions(+), 29 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 2e9b45f9..dddd3cd6 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -1209,14 +1209,10 @@ set_pgpolicy(vector strvec, void *ptr, const
> char *file, int line_nr)
>  int
>  print_pgpolicy(struct strbuf *buff, long pgpolicy)
>  {
> -       char str[POLICY_NAME_SIZE];
> -
>         if (!pgpolicy)
>                 return 0;
>  
> -       get_pgpolicy_name(str, POLICY_NAME_SIZE, pgpolicy);
> -
> -       return append_strbuf_quoted(buff, str);
> +       return append_strbuf_quoted(buff,
> get_pgpolicy_name(pgpolicy));
>  }
>  
>  declare_def_handler(pgpolicy, set_pgpolicy)
> diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
> index e14da8cc..edc3c611 100644
> --- a/libmultipath/pgpolicies.c
> +++ b/libmultipath/pgpolicies.c
> @@ -31,34 +31,23 @@ int get_pgpolicy_id(char * str)
>         return IOPOLICY_UNDEF;
>  }
>  
> -int get_pgpolicy_name(char * buff, int len, int id)
> +const char *get_pgpolicy_name(int id)
>  {
> -       char * s;
> -
>         switch (id) {
>         case FAILOVER:
> -               s = "failover";
> -               break;
> +               return "failover";
>         case MULTIBUS:
> -               s = "multibus";
> -               break;
> +               return "multibus";
>         case GROUP_BY_SERIAL:
> -               s = "group_by_serial";
> -               break;
> +               return "group_by_serial";
>         case GROUP_BY_PRIO:
> -               s = "group_by_prio";
> -               break;
> +               return "group_by_prio";
>         case GROUP_BY_NODE_NAME:
> -               s = "group_by_node_name";
> -               break;
> +               return "group_by_node_name";
>         case GROUP_BY_TPG:
> -               s = "group_by_tpg";
> -               break;
> -       default:
> -               s = "undefined";
> -               break;
> +               return "group_by_tpg";
>         }
> -       return snprintf(buff, len, "%s", s);
> +       return "undefined"; /* IOPOLICY_UNDEF */
>  }
>  
>  
> diff --git a/libmultipath/pgpolicies.h b/libmultipath/pgpolicies.h
> index d3ab2f35..9e4ddda2 100644
> --- a/libmultipath/pgpolicies.h
> +++ b/libmultipath/pgpolicies.h
> @@ -21,7 +21,7 @@ enum iopolicies {
>  };
>  
>  int get_pgpolicy_id(char *);
> -int get_pgpolicy_name (char *, int, int);
> +const char *get_pgpolicy_name (int);
>  int group_paths(struct multipath *, int);
>  /*
>   * policies
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index 841fa247..d214281b 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -268,7 +268,6 @@ verify_alua_prio(struct multipath *mp)
>  int select_pgpolicy(struct config *conf, struct multipath * mp)
>  {
>         const char *origin;
> -       char buff[POLICY_NAME_SIZE];
>         int log_prio = 3;
>  
>         if (conf->pgpolicy_flag > 0) {
> @@ -288,9 +287,8 @@ out:
>                 log_prio = 1;
>         }
>         mp->pgpolicyfn = pgpolicies[mp->pgpolicy];
> -       get_pgpolicy_name(buff, POLICY_NAME_SIZE, mp->pgpolicy);
> -       condlog(log_prio, "%s: path_grouping_policy = %s %s", mp-
> >alias, buff,
> -               origin);
> +       condlog(log_prio, "%s: path_grouping_policy = %s %s", mp-
> >alias,
> +               get_pgpolicy_name(mp->pgpolicy), origin);
>         return 0;
>  }
>