On Fri, Mar 29, 2024 at 11:10:54AM +0000, le...@solinno.co.uk 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
