On Thu, Nov 04, 2021 at 08:34:33PM +0000, Martin Wilck wrote:
> 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 <bmarz...@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?

Yeah. all the uint cases accepted the entire UINT range.  As you've
seen, I end up using the int range checking for more functions later.
 
> 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.

Sure.

-Ben

> 
> >  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,

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

Reply via email to