Re: [PATCH] tools/xentop: reject invalid --delay and --iterations arguments

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Aug 05, 2026 at 09:55:21PM +0800, Matthias Goergens wrote:
> xentop parses -d/--delay and -i/--iterations with atoi(), so invalid
> input is silently accepted with surprising results: a fractional delay
> such as "-d 2.5" is truncated to 2; "-d abc" parses as 0, which in
> batch mode turns the output loop into a busy loop; "-d -1" wraps to an
> effective delay of about 136 years; and "-i 0" (or any unparsable
> iterations count) decrements an unsigned counter from zero, running
> for about 2^32 iterations.  atoi() also has undefined behaviour on
> out-of-range input.
> 
> Parse both options with strtoull() instead, and reject anything that
> is not a plain decimal integer in range: a sign, a fractional part,
> trailing junk or overflow now produce an error and exit rather than a
> silently wrong value.
> 
> Compatibility considerations: "--delay 0" remains accepted, since
> updating as fast as possible is a plausible deliberate choice and
> works today; "--iterations 0" is rejected, since running the loop
> 2^32 times cannot be what the caller meant.  The interactive 'D'
> prompt already validates its input and is unchanged.  The only
> previously useful invocation this breaks is a fractional delay, which
> now fails loudly instead of silently rounding down - which is the
> point of the change.
> 
> A patch documenting the --delay truncation was posted in 2010 but
> never applied:
> Link: 
> https://lore.kernel.org/xen-devel/01ea26d2420e3562eb30.1292604768@xxxxxxxxxxxxxxxxxxxxxxxxxx/
> 
> Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
> ---
> Tested by compiling with -Wall -Wextra (no new warnings) and by running
> the parse helper, extracted verbatim from the patched file, against a
> 19-case input matrix covering both the accepted and the rejected inputs
> listed above.  Not run against a live Xen host: the change is confined
> to command line parsing, ahead of any hypervisor interaction.
> 
> Happy to add a CHANGELOG.md entry under "Changed" if that is wanted for
> a tools CLI change of this size.

No, I don't think it's necessary, this patch is just error handling, and
a bit of clarification I suppose.

Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech