Re: [PATCH v7 3/5] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi()

From: Sean Christopherson
Date: Mon Oct 31 2022 - 15:17:45 EST


On Mon, Oct 31, 2022, Vipin Sharma wrote:
> atoi() doesn't detect errors. There is no way to know that a 0 return
> is correct conversion or due to an error.
>
> Introduce atoi_paranoid() to detect errors and provide correct
> conversion. Replace all atoi() calls with atoi_paranoid().
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 6d23878bbfe1..ec0f070a6f21 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -334,3 +334,22 @@ long get_run_delay(void)
>
> return val[1];
> }
> +
> +int atoi_paranoid(const char *num_str)
> +{
> + char *end_ptr;
> + long num;
> +
> + errno = 0;
> + num = strtol(num_str, &end_ptr, 10);

I take back my review. This forces specifying params in decimal, e.g. a large
hex number yields:

strtol("0xffffffffff") failed to parse trailing characters "xffffffffff".

Obviously I'm intentionally being a bad user in this particular case, but there
will inevitably be tests that want to take hex input, e.g. an x86 test that takes
an MSR index would definitely want hex input.

Looking through all selftests, I don't think there are existing cases that would
likely want hex, but it's trivial to support since strtol() will autodetect the
format if the base is '0', i.e.

diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 210e98a49a83..8c37cfa12edc 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -341,7 +341,7 @@ int atoi_paranoid(const char *num_str)
long num;

errno = 0;
- num = strtol(num_str, &end_ptr, 10);
+ num = strtol(num_str, &end_ptr, 0);
TEST_ASSERT(!errno, "strtol(\"%s\") failed", num_str);
TEST_ASSERT(num_str != end_ptr,
"strtol(\"%s\") didn't find a valid integer.\n", num_str);