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@xxxxxxxxxx> 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 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel