[dm-devel] [PATCH v2 8/9] libmultipath: split set_int to enable reuse

[Martin Wilck]

On Wed, 2021-11-10 at 19:06 -0600, Benjamin Marzinski wrote:
> Split the code that does the actual value parsing out of set_int(),
> into
> a helper function, do_set_int(), so that it can be used by other
> handlers. These functions no longer set the config value at all, when
> they have invalid input.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>

I have to apologize. 

My review on the v1 of this patch suggested that a value like "0   "
was possible in the setter functions in dict.c. I double-checked the
parser code, and that was wrong. The parser already strips trailing
whitespace. We actually have a test for that in tests/parser.c
(test04).

So, I have to say I actually prefer the v1, which was leaner and worked
just as well as this one. My concern was that you didn't set any value
(relying on the previously-set default), but you've convinced me that
that was ok.

So, if you don't mind, I'll put my "Reviewed-by" on the v1 of this
patch.

Regards,
Martin

> ---
>  libmultipath/dict.c | 92 ++++++++++++++++++++++++++-----------------
> --
>  1 file changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 1b4e1106..3212d14c 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> 
> ...
>  int
> @@ -1230,6 +1238,7 @@ declare_mp_snprint(pgfailback,
> print_pgfailback)
>  static int
>  no_path_retry_helper(vector strvec, void *ptr, const char *file, int
> line_nr)
>  {
> +       int rc = 0;
>         int *int_ptr = (int *)ptr;
>         char * buff;
>  
> @@ -1237,15 +1246,18 @@ no_path_retry_helper(vector strvec, void
> *ptr, const char *file, int line_nr)
>         if (!buff)
>                 return 1;
>  
> -       if (!strcmp(buff, "fail") || !strcmp(buff, "0"))
> +       if (!strcmp(buff, "fail"))
>                 *int_ptr = NO_PATH_RETRY_FAIL;
>         else if (!strcmp(buff, "queue"))
>                 *int_ptr = NO_PATH_RETRY_QUEUE;
> -       else if ((*int_ptr = atoi(buff)) < 1)
> -               *int_ptr = NO_PATH_RETRY_UNDEF;
> +       else {
> +               rc = do_set_int(strvec, ptr, 0, INT_MAX, file,
> line_nr, buff);
> +               if (rc == 0 && *int_ptr == 0)
> +                       *int_ptr = NO_PATH_RETRY_FAIL;
> +       }
>  




>         FREE(buff);
> -       return 0;
> +       return rc;
>  }
>  
>  int
> @@ -1387,6 +1399,7 @@ snprint_mp_reservation_key (struct config
> *conf, struct strbuf *buff,
>  static int
>  set_off_int_undef(vector strvec, void *ptr, const char *file, int
> line_nr)
>  {
> +       int rc =0;
>         int *int_ptr = (int *)ptr;
>         char * buff;
>  
> @@ -1394,13 +1407,16 @@ set_off_int_undef(vector strvec, void *ptr,
> const char *file, int line_nr)
>         if (!buff)
>                 return 1;
>  
> -       if (!strcmp(buff, "no") || !strcmp(buff, "0"))
> +       if (!strcmp(buff, "no"))
>                 *int_ptr = NU_NO;
> -       else if ((*int_ptr = atoi(buff)) < 1)
> -               *int_ptr = NU_UNDEF;
> +       else {
> +               rc = do_set_int(strvec, ptr, 0, INT_MAX, file,
> line_nr, buff);
> +               if (rc == 0 && *int_ptr == 0)
> +                       *int_ptr = NU_NO;
> +       }
>  
>         FREE(buff);
> -       return 0;
> +       return rc;
>  }
>  
>  int