[dm-devel] [PATCH 5/8] libmultipath: make set_int take a range for valid values

[Martin Wilck]

On Wed, 2021-10-06 at 15:04 -0500, Benjamin Marzinski wrote:
> If a value outside of the valid range is passed to set_int, it caps
> the
> value at appropriate limit, and issues a warning.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>

One nitpick below.

It's a lot of code changes for just two cases where we have nontrivial
values for min and max. I guess for uint the count of nontrivial cases
was zero?

Yet it's an improvement, so I'm going to ack when the nit is fixed.

Martin



> ---
>  libmultipath/dict.c | 121 +++++++++++++++++++++++++++---------------
> --
>  1 file changed, 75 insertions(+), 46 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index eb2c44c0..1758bd26 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> ...
> @@ -298,7 +347,7 @@ declare_def_snprint(checkint, print_int)
>  declare_def_handler(max_checkint, set_uint)
>  declare_def_snprint(max_checkint, print_int)
>  
> -declare_def_handler(verbosity, set_int)
> +declare_def_range_handler(verbosity, 0, 4)

You should use MAX_VERBOSITY here.


>  declare_def_snprint(verbosity, print_int)
>  
>  declare_def_handler(reassign_maps, set_yes_no)
> @@ -473,22 +522,22 @@ declare_ovr_snprint(checker_name, print_str)
>  declare_hw_handler(checker_name, set_str)
>  declare_hw_snprint(checker_name, print_str)
>  
> -declare_def_handler(minio, set_int)
> +declare_def_range_handler(minio, 0, INT_MAX)
>  declare_def_snprint_defint(minio, print_int, DEFAULT_MINIO)
> -declare_ovr_handler(minio, set_int)
> +declare_ovr_range_handler(minio, 0, INT_MAX)
>  declare_ovr_snprint(minio, print_nonzero)
> -declare_hw_handler(minio, set_int)
> +declare_hw_range_handler(minio, 0, INT_MAX)
>  declare_hw_snprint(minio, print_nonzero)
> -declare_mp_handler(minio, set_int)
> +declare_mp_range_handler(minio, 0, INT_MAX)
>  declare_mp_snprint(minio, print_nonzero)
>  
> -declare_def_handler(minio_rq, set_int)
> +declare_def_range_handler(minio_rq, 0, INT_MAX)
>  declare_def_snprint_defint(minio_rq, print_int, DEFAULT_MINIO_RQ)
> -declare_ovr_handler(minio_rq, set_int)
> +declare_ovr_range_handler(minio_rq, 0, INT_MAX)
>  declare_ovr_snprint(minio_rq, print_nonzero)
> -declare_hw_handler(minio_rq, set_int)
> +declare_hw_range_handler(minio_rq, 0, INT_MAX)
>  declare_hw_snprint(minio_rq, print_nonzero)
> -declare_mp_handler(minio_rq, set_int)
> +declare_mp_range_handler(minio_rq, 0, INT_MAX)
>  declare_mp_snprint(minio_rq, print_nonzero)
>  
>  declare_def_handler(queue_without_daemon, set_yes_no)
> @@ -512,7 +561,7 @@ snprint_def_queue_without_daemon(struct config
> *conf, struct strbuf *buff,
>         return append_strbuf_quoted(buff, qwd);
>  }
>  
> -declare_def_handler(checker_timeout, set_int)
> +declare_def_range_handler(checker_timeout, 0, INT_MAX)
>  declare_def_snprint(checker_timeout, print_nonzero)
>  
>  declare_def_handler(allow_usb_devices, set_yes_no)
> @@ -583,13 +632,13 @@ declare_hw_snprint(deferred_remove,
> print_yes_no_undef)
>  declare_mp_handler(deferred_remove, set_yes_no_undef)
>  declare_mp_snprint(deferred_remove, print_yes_no_undef)
>  
> -declare_def_handler(retrigger_tries, set_int)
> +declare_def_range_handler(retrigger_tries, 0, INT_MAX)
>  declare_def_snprint(retrigger_tries, print_int)
>  
> -declare_def_handler(retrigger_delay, set_int)
> +declare_def_range_handler(retrigger_delay, 0, INT_MAX)
>  declare_def_snprint(retrigger_delay, print_int)
>  
> -declare_def_handler(uev_wait_timeout, set_int)
> +declare_def_range_handler(uev_wait_timeout, 0, INT_MAX)
>  declare_def_snprint(uev_wait_timeout, print_int)
>  
>  declare_def_handler(strict_timing, set_yes_no)
> @@ -616,19 +665,19 @@ static int
> snprint_def_disable_changed_wwids(struct config *conf,
>         return print_ignored(buff);
>  }
>  
> -declare_def_handler(remove_retries, set_int)
> +declare_def_range_handler(remove_retries, 0, INT_MAX)
>  declare_def_snprint(remove_retries, print_int)
>  
> -declare_def_handler(max_sectors_kb, set_int)
> +declare_def_range_handler(max_sectors_kb, 0, INT_MAX)
>  declare_def_snprint(max_sectors_kb, print_nonzero)
> -declare_ovr_handler(max_sectors_kb, set_int)
> +declare_ovr_range_handler(max_sectors_kb, 0, INT_MAX)
>  declare_ovr_snprint(max_sectors_kb, print_nonzero)
> -declare_hw_handler(max_sectors_kb, set_int)
> +declare_hw_range_handler(max_sectors_kb, 0, INT_MAX)
>  declare_hw_snprint(max_sectors_kb, print_nonzero)
> -declare_mp_handler(max_sectors_kb, set_int)
> +declare_mp_range_handler(max_sectors_kb, 0, INT_MAX)
>  declare_mp_snprint(max_sectors_kb, print_nonzero)
>  
> -declare_def_handler(find_multipaths_timeout, set_int)
> +declare_def_range_handler(find_multipaths_timeout, INT_MIN, INT_MAX)
>  declare_def_snprint_defint(find_multipaths_timeout, print_int,
>                            DEFAULT_FIND_MULTIPATHS_TIMEOUT)
>  
> @@ -1385,27 +1434,7 @@ declare_ovr_snprint(recheck_wwid,
> print_yes_no_undef)
>  declare_hw_handler(recheck_wwid, set_yes_no_undef)
>  declare_hw_snprint(recheck_wwid, print_yes_no_undef)
>  
> -
> -static int
> -def_uxsock_timeout_handler(struct config *conf, vector strvec, const
> char *file,
> -                          int line_nr)
> -{
> -       unsigned int uxsock_timeout;
> -       char *buff;
> -
> -       buff = set_value(strvec);
> -       if (!buff)
> -               return 1;
> -
> -       if (sscanf(buff, "%u", &uxsock_timeout) == 1 &&
> -           uxsock_timeout > DEFAULT_REPLY_TIMEOUT)
> -               conf->uxsock_timeout = uxsock_timeout;
> -       else
> -               conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
> -
> -       free(buff);
> -       return 0;
> -}
> +declare_def_range_handler(uxsock_timeout, DEFAULT_REPLY_TIMEOUT,
> INT_MAX)
>  
>  static int
>  hw_vpd_vendor_handler(struct config *conf, vector strvec, const char
> *file,