[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 4/6] tools/misc: xenwatchdogd: add parse_secs()
On Fri, Mar 29, 2024 at 11:10:54AM +0000, leigh@xxxxxxxxxxxxx wrote: > diff --git a/tools/misc/xenwatchdogd.c b/tools/misc/xenwatchdogd.c > index 2e7f9f51c5..19ec4c5359 100644 > --- a/tools/misc/xenwatchdogd.c > +++ b/tools/misc/xenwatchdogd.c > @@ -49,6 +49,18 @@ static void catch_usr1(int sig) > done = true; > } > > +static int parse_secs(const char *arg, const char *what) > +{ > + char *endptr; > + unsigned long val; > + > + val = strtoul(arg, &endptr, 0); > + if (val > INT_MAX || *endptr != 0) nit: '\0' would be more accurate ^ here. Otherwise, just `*endptr` without "!= 0" would work too. But comparing a char to a digit feels wrong. > + errx(EXIT_FAILURE, "invalid %s: '%s'", what, arg); > + > + return val; > +} > + > int main(int argc, char **argv) > { > int id; > @@ -64,16 +76,11 @@ int main(int argc, char **argv) > if (h == NULL) > err(EXIT_FAILURE, "xc_interface_open"); > > - t = strtoul(argv[1], NULL, 0); > - if (t == ULONG_MAX) > - err(EXIT_FAILURE, "strtoul"); > + t = parse_secs(argv[optind], "timeout"); > > s = t / 2; > - if (argc == 3) { > - s = strtoul(argv[2], NULL, 0); > - if (s == ULONG_MAX) > - err(EXIT_FAILURE, "strtoul"); > - } > + if (argc == 3) > + s = parse_secs(argv[optind], "sleep"); This is parsing the wrong value here, should it be +1 to parse the sleep argument instead of the timeout argument a second time. Also, are you sure you want to start using "optind" here? It's a variable used by getopt(), but we don't use it yet. It almost feel like "optind" happen to be set to 1 by luck. Thanks, -- Anthony PERARD
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |