[v7,4/5] KVM: selftests: Add atoi_positive() and atoi_non_negative() for input validation
Commit Message
Many KVM selftests take command line arguments which are supposed to be
positive (>0) or non-negative (>=0). Some tests do these validation and
some missed adding the check.
Add atoi_positive() and atoi_non_negative() to validate inputs in
selftests before proceeding to use those values.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/aarch64/arch_timer.c | 25 ++++---------------
.../selftests/kvm/aarch64/debug-exceptions.c | 2 +-
.../testing/selftests/kvm/aarch64/vgic_irq.c | 2 +-
.../selftests/kvm/access_tracking_perf_test.c | 2 +-
.../selftests/kvm/demand_paging_test.c | 4 +--
.../selftests/kvm/dirty_log_perf_test.c | 12 ++++-----
.../testing/selftests/kvm/include/test_util.h | 2 ++
.../selftests/kvm/kvm_page_table_test.c | 4 +--
tools/testing/selftests/kvm/lib/test_util.c | 16 ++++++++++++
.../selftests/kvm/max_guest_memory_test.c | 7 +++---
.../kvm/memslot_modification_stress_test.c | 6 ++---
.../testing/selftests/kvm/memslot_perf_test.c | 22 ++++------------
.../selftests/kvm/set_memory_region_test.c | 2 +-
13 files changed, 47 insertions(+), 59 deletions(-)
Comments
On Mon, Oct 31, 2022, Vipin Sharma wrote:
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index ec0f070a6f21..210e98a49a83 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -353,3 +353,19 @@ int atoi_paranoid(const char *num_str)
>
> return num;
> }
> +
> +uint32_t atoi_positive(const char *num_str)
I think it makes sense to inline atoi_positive() and atoi_non_negative() in
test_util.h. Depending on developer's setups, it might be one less layer to jump
through to look at the implementation.
> +{
> + int num = atoi_paranoid(num_str);
> +
> + TEST_ASSERT(num > 0, "%s is not a positive integer.\n", num_str);
Newlines aren't needed in asserts. This applies to atoi_paranoid() in the previous
patch as well (I initially missed them).
> + return num;
> +}
> +
> +uint32_t atoi_non_negative(const char *num_str)
> +{
> + int num = atoi_paranoid(num_str);
> +
> + TEST_ASSERT(num >= 0, "%s is not a non-negative integer.\n", num_str);
> + return num;
> +}
> diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/max_guest_memory_test.c
> index 1595b73dc09a..20015de3b91c 100644
> --- a/tools/testing/selftests/kvm/max_guest_memory_test.c
> +++ b/tools/testing/selftests/kvm/max_guest_memory_test.c
> @@ -193,15 +193,14 @@ int main(int argc, char *argv[])
> while ((opt = getopt(argc, argv, "c:h:m:s:H")) != -1) {
> switch (opt) {
> case 'c':
> - nr_vcpus = atoi_paranoid(optarg);
> - TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
> + nr_vcpus = atoi_positive(optarg);
I know I originally made the claim that the assert would provide enough context
to offest lack of a specific message, but after actually playing around with this,
past me was wrong. E.g. this
Memory size must be greater than 0, got '-1'
is much more helpful than
-1 is not a positive integer.
E.g. something like this?
static inline uint32_t atoi_positive(const char *name, const char *num_str)
{
int num = atoi_paranoid(num_str);
TEST_ASSERT(num > 0, "%s must be greater than 0, got '%s'", name, num_str);
return num;
}
static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
{
int num = atoi_paranoid(num_str);
TEST_ASSERT(num >= 0, "%s must be non-negative, got '%s'", name, num_str);
return num;
}
IMO, that also makes the code slightly easier to follow as it's super obvious
what is being parsed.
p.wr_fract = atoi_positive("Write fraction", optarg);
p.iterations = atoi_positive("Number of iterations", optarg);
nr_vcpus = atoi_positive("Number of vCPUs", optarg);
Last thought: my vote would be to ignore the 80 char soft limit when adding the
"name" to these calls, in every case except nr_memslot_modifications the overrun
is relatively minor and not worth wrapping. See below for my thougts on that one.
> break;
> case 'm':
> - max_mem = atoi_paranoid(optarg) * size_1gb;
> + max_mem = atoi_positive(optarg) * size_1gb;
> TEST_ASSERT(max_mem > 0, "memory size must be >0");
This assert can be dropped, max_mem is a uint64_t so wrapping to '0' is impossible.
> break;
> case 's':
> - slot_size = atoi_paranoid(optarg) * size_1gb;
> + slot_size = atoi_positive(optarg) * size_1gb;
Same thing here.
> TEST_ASSERT(slot_size > 0, "slot size must be >0");
> break;
> case 'H':
> diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> index 865276993ffb..7539ee7b6e95 100644
> --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> @@ -175,7 +175,7 @@ int main(int argc, char *argv[])
> p.partition_vcpu_memory_access = false;
> break;
memslot_modification_delay can be converted to atoi_non_negative(), it open codes
strtoul(), but the "long" part is unnecessary because memslot_modification_delay
is an "unsigned int", not an "unsigned long".
> case 'i':
> - p.nr_memslot_modifications = atoi_paranoid(optarg);
> + p.nr_memslot_modifications = atoi_positive(optarg);
To avoid a ridiculously long line, my vote is to rename the test args. The names
are rather odd irrespective of line length. E.g. in a prep patch do
s/memslot_modification_delay/delay
s/nr_memslot_modifications/nr_iterations
which yields parsing of:
while ((opt = getopt(argc, argv, "hm:d:b:v:oi:")) != -1) {
switch (opt) {
case 'm':
guest_modes_cmdline(optarg);
break;
case 'd':
p.delay = atoi_non_negative("Delay", optarg);
break;
case 'b':
guest_percpu_mem_size = parse_size(optarg);
break;
case 'v':
nr_vcpus = atoi_positive("Number of vCPUs", optarg);
TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d",
max_vcpus);
break;
case 'o':
p.partition_vcpu_memory_access = false;
break;
case 'i':
p.nr_iterations = atoi_positive("Number of iterations", optarg);
break;
case 'h':
default:
help(argv[0]);
break;
}
}
On Mon, Oct 31, 2022 at 12:48 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 31, 2022, Vipin Sharma wrote:
> > diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> > index ec0f070a6f21..210e98a49a83 100644
> > --- a/tools/testing/selftests/kvm/lib/test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/test_util.c
> > @@ -353,3 +353,19 @@ int atoi_paranoid(const char *num_str)
> >
> > return num;
> > }
> > +
> > +uint32_t atoi_positive(const char *num_str)
>
> I think it makes sense to inline atoi_positive() and atoi_non_negative() in
> test_util.h. Depending on developer's setups, it might be one less layer to jump
> through to look at the implementation.
>
I am not sure if this makes life much easier for developers, as
"inline" can totally be ignored by the compiler. Also, not sure how
much qualitative improvement it will add in the developer's code
browsing journey. Anyways, I will add "inline" in the next version.
> > +{
> > + int num = atoi_paranoid(num_str);
> > +
> > + TEST_ASSERT(num > 0, "%s is not a positive integer.\n", num_str);
>
> Newlines aren't needed in asserts. This applies to atoi_paranoid() in the previous
> patch as well (I initially missed them).
>
Okay, I will remove it from the previous patch also.
> > + return num;
> > +}
> > +
> > +uint32_t atoi_non_negative(const char *num_str)
> > +{
> > + int num = atoi_paranoid(num_str);
> > +
> > + TEST_ASSERT(num >= 0, "%s is not a non-negative integer.\n", num_str);
> > + return num;
> > +}
> > diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/max_guest_memory_test.c
> > index 1595b73dc09a..20015de3b91c 100644
> > --- a/tools/testing/selftests/kvm/max_guest_memory_test.c
> > +++ b/tools/testing/selftests/kvm/max_guest_memory_test.c
> > @@ -193,15 +193,14 @@ int main(int argc, char *argv[])
> > while ((opt = getopt(argc, argv, "c:h:m:s:H")) != -1) {
> > switch (opt) {
> > case 'c':
> > - nr_vcpus = atoi_paranoid(optarg);
> > - TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
> > + nr_vcpus = atoi_positive(optarg);
>
> I know I originally made the claim that the assert would provide enough context
> to offest lack of a specific message, but after actually playing around with this,
> past me was wrong. E.g. this
>
> Memory size must be greater than 0, got '-1'
>
> is much more helpful than
>
> -1 is not a positive integer.
>
> E.g. something like this?
>
> static inline uint32_t atoi_positive(const char *name, const char *num_str)
> {
> int num = atoi_paranoid(num_str);
>
> TEST_ASSERT(num > 0, "%s must be greater than 0, got '%s'", name, num_str);
> return num;
> }
>
> static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
> {
> int num = atoi_paranoid(num_str);
>
> TEST_ASSERT(num >= 0, "%s must be non-negative, got '%s'", name, num_str);
> return num;
> }
>
> IMO, that also makes the code slightly easier to follow as it's super obvious
> what is being parsed.
>
> p.wr_fract = atoi_positive("Write fraction", optarg);
>
> p.iterations = atoi_positive("Number of iterations", optarg);
>
> nr_vcpus = atoi_positive("Number of vCPUs", optarg);
>
I will make this change. It is indeed better.
> Last thought: my vote would be to ignore the 80 char soft limit when adding the
> "name" to these calls, in every case except nr_memslot_modifications the overrun
> is relatively minor and not worth wrapping. See below for my thougts on that one.
>
> > break;
> > case 'm':
> > - max_mem = atoi_paranoid(optarg) * size_1gb;
> > + max_mem = atoi_positive(optarg) * size_1gb;
> > TEST_ASSERT(max_mem > 0, "memory size must be >0");
>
> This assert can be dropped, max_mem is a uint64_t so wrapping to '0' is impossible.
>
I intentionally kept it, as it is also protecting against having
accidently making size_1gb to 0.
> > break;
> > case 's':
> > - slot_size = atoi_paranoid(optarg) * size_1gb;
> > + slot_size = atoi_positive(optarg) * size_1gb;
>
> Same thing here.
>
> > TEST_ASSERT(slot_size > 0, "slot size must be >0");
> > break;
> > case 'H':
> > diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > index 865276993ffb..7539ee7b6e95 100644
> > --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > @@ -175,7 +175,7 @@ int main(int argc, char *argv[])
> > p.partition_vcpu_memory_access = false;
> > break;
>
> memslot_modification_delay can be converted to atoi_non_negative(), it open codes
> strtoul(), but the "long" part is unnecessary because memslot_modification_delay
> is an "unsigned int", not an "unsigned long".
>
> > case 'i':
> > - p.nr_memslot_modifications = atoi_paranoid(optarg);
> > + p.nr_memslot_modifications = atoi_positive(optarg);
>
> To avoid a ridiculously long line, my vote is to rename the test args. The names
> are rather odd irrespective of line length. E.g. in a prep patch do
>
> s/memslot_modification_delay/delay
> s/nr_memslot_modifications/nr_iterations
>
Okay, I will change this and any other places I find which can be shortened.
> which yields parsing of:
>
> while ((opt = getopt(argc, argv, "hm:d:b:v:oi:")) != -1) {
> switch (opt) {
> case 'm':
> guest_modes_cmdline(optarg);
> break;
> case 'd':
> p.delay = atoi_non_negative("Delay", optarg);
> break;
> case 'b':
> guest_percpu_mem_size = parse_size(optarg);
> break;
> case 'v':
> nr_vcpus = atoi_positive("Number of vCPUs", optarg);
> TEST_ASSERT(nr_vcpus <= max_vcpus,
> "Invalid number of vcpus, must be between 1 and %d",
> max_vcpus);
> break;
> case 'o':
> p.partition_vcpu_memory_access = false;
> break;
> case 'i':
> p.nr_iterations = atoi_positive("Number of iterations", optarg);
> break;
> case 'h':
> default:
> help(argv[0]);
> break;
> }
> }
>
On Tue, Nov 01, 2022, Vipin Sharma wrote:
> On Mon, Oct 31, 2022 at 12:48 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Oct 31, 2022, Vipin Sharma wrote:
> > > diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> > > index ec0f070a6f21..210e98a49a83 100644
> > > --- a/tools/testing/selftests/kvm/lib/test_util.c
> > > +++ b/tools/testing/selftests/kvm/lib/test_util.c
> > > @@ -353,3 +353,19 @@ int atoi_paranoid(const char *num_str)
> > >
> > > return num;
> > > }
> > > +
> > > +uint32_t atoi_positive(const char *num_str)
> >
> > I think it makes sense to inline atoi_positive() and atoi_non_negative() in
> > test_util.h. Depending on developer's setups, it might be one less layer to jump
> > through to look at the implementation.
> >
>
> I am not sure if this makes life much easier for developers, as
> "inline" can totally be ignored by the compiler. Also, not sure how
> much qualitative improvement it will add in the developer's code
> browsing journey. Anyways, I will add "inline" in the next version.
To be clear, it's not about adding "inline", it's about not having separate
declarations and definitions. E.g. I've yet to achieve a setup that has 100%
accuracy when it comes to navigating to a definition versus a declaration. And
when poking around code, seeing a "static inline" function provides a hint that
a function is likely a simple wrapper without even having to look at the
implementation.
These are all small things, but I can't think of a reason _not_ to inline these
trivial wrappers.
> > Last thought: my vote would be to ignore the 80 char soft limit when adding the
> > "name" to these calls, in every case except nr_memslot_modifications the overrun
> > is relatively minor and not worth wrapping. See below for my thougts on that one.
> >
> > > break;
> > > case 'm':
> > > - max_mem = atoi_paranoid(optarg) * size_1gb;
> > > + max_mem = atoi_positive(optarg) * size_1gb;
> > > TEST_ASSERT(max_mem > 0, "memory size must be >0");
> >
> > This assert can be dropped, max_mem is a uint64_t so wrapping to '0' is impossible.
> >
>
> I intentionally kept it, as it is also protecting against having
> accidently making size_1gb to 0.
Heh, the test has far, far bigger problems if it screws up size_1gb. And that's
an orthogonal concern as the test would be horrifically broken regardless of
whether or not the user specified '-m' and/or '-s'.
A better approach is to replace the homebrewed size_1gb with SZ_1G from
tools/include/linux/sizes.h. I, and many others, completely overlooked size.h.
On Tue, Nov 1, 2022 at 12:20 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Nov 01, 2022, Vipin Sharma wrote:
> > On Mon, Oct 31, 2022 at 12:48 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Mon, Oct 31, 2022, Vipin Sharma wrote:
> > > > diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> > > > index ec0f070a6f21..210e98a49a83 100644
> > > > --- a/tools/testing/selftests/kvm/lib/test_util.c
> > > > +++ b/tools/testing/selftests/kvm/lib/test_util.c
> > > > @@ -353,3 +353,19 @@ int atoi_paranoid(const char *num_str)
> > > >
> > > > return num;
> > > > }
> > > > +
> > > > +uint32_t atoi_positive(const char *num_str)
> > >
> > > I think it makes sense to inline atoi_positive() and atoi_non_negative() in
> > > test_util.h. Depending on developer's setups, it might be one less layer to jump
> > > through to look at the implementation.
> > >
> >
> > I am not sure if this makes life much easier for developers, as
> > "inline" can totally be ignored by the compiler. Also, not sure how
> > much qualitative improvement it will add in the developer's code
> > browsing journey. Anyways, I will add "inline" in the next version.
>
> To be clear, it's not about adding "inline", it's about not having separate
> declarations and definitions. E.g. I've yet to achieve a setup that has 100%
> accuracy when it comes to navigating to a definition versus a declaration. And
> when poking around code, seeing a "static inline" function provides a hint that
> a function is likely a simple wrapper without even having to look at the
> implementation.
>
> These are all small things, but I can't think of a reason _not_ to inline these
> trivial wrappers.
>
Note to myself: Read the whole sentence!
I skipped "in test_util.h". Got it.
> > > Last thought: my vote would be to ignore the 80 char soft limit when adding the
> > > "name" to these calls, in every case except nr_memslot_modifications the overrun
> > > is relatively minor and not worth wrapping. See below for my thougts on that one.
> > >
> > > > break;
> > > > case 'm':
> > > > - max_mem = atoi_paranoid(optarg) * size_1gb;
> > > > + max_mem = atoi_positive(optarg) * size_1gb;
> > > > TEST_ASSERT(max_mem > 0, "memory size must be >0");
> > >
> > > This assert can be dropped, max_mem is a uint64_t so wrapping to '0' is impossible.
> > >
> >
> > I intentionally kept it, as it is also protecting against having
> > accidently making size_1gb to 0.
>
> Heh, the test has far, far bigger problems if it screws up size_1gb. And that's
> an orthogonal concern as the test would be horrifically broken regardless of
> whether or not the user specified '-m' and/or '-s'.
>
> A better approach is to replace the homebrewed size_1gb with SZ_1G from
> tools/include/linux/sizes.h. I, and many others, completely overlooked size.h.
I will replace it.
@@ -414,36 +414,21 @@ static bool parse_args(int argc, char *argv[])
while ((opt = getopt(argc, argv, "hn:i:p:m:")) != -1) {
switch (opt) {
case 'n':
- test_args.nr_vcpus = atoi_paranoid(optarg);
- if (test_args.nr_vcpus <= 0) {
- pr_info("Positive value needed for -n\n");
- goto err;
- } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+ test_args.nr_vcpus = atoi_positive(optarg);
+ if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
pr_info("Max allowed vCPUs: %u\n",
KVM_MAX_VCPUS);
goto err;
}
break;
case 'i':
- test_args.nr_iter = atoi_paranoid(optarg);
- if (test_args.nr_iter <= 0) {
- pr_info("Positive value needed for -i\n");
- goto err;
- }
+ test_args.nr_iter = atoi_positive(optarg);
break;
case 'p':
- test_args.timer_period_ms = atoi_paranoid(optarg);
- if (test_args.timer_period_ms <= 0) {
- pr_info("Positive value needed for -p\n");
- goto err;
- }
+ test_args.timer_period_ms = atoi_positive(optarg);
break;
case 'm':
- test_args.migration_freq_ms = atoi_paranoid(optarg);
- if (test_args.migration_freq_ms < 0) {
- pr_info("0 or positive value needed for -m\n");
- goto err;
- }
+ test_args.migration_freq_ms = atoi_non_negative(optarg);
break;
case 'h':
default:
@@ -423,7 +423,7 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "i:")) != -1) {
switch (opt) {
case 'i':
- ss_iteration = atoi_paranoid(optarg);
+ ss_iteration = atoi_positive(optarg);
break;
case 'h':
default:
@@ -824,7 +824,7 @@ int main(int argc, char **argv)
while ((opt = getopt(argc, argv, "hn:e:l:")) != -1) {
switch (opt) {
case 'n':
- nr_irqs = atoi_paranoid(optarg);
+ nr_irqs = atoi_non_negative(optarg);
if (nr_irqs > 1024 || nr_irqs % 32)
help(argv[0]);
break;
@@ -368,7 +368,7 @@ int main(int argc, char *argv[])
params.vcpu_memory_bytes = parse_size(optarg);
break;
case 'v':
- params.nr_vcpus = atoi_paranoid(optarg);
+ params.nr_vcpus = atoi_positive(optarg);
break;
case 'o':
overlap_memory_access = true;
@@ -427,8 +427,8 @@ int main(int argc, char *argv[])
p.src_type = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive(optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 'o':
@@ -416,9 +416,7 @@ int main(int argc, char *argv[])
run_vcpus_while_disabling_dirty_logging = true;
break;
case 'f':
- p.wr_fract = atoi_paranoid(optarg);
- TEST_ASSERT(p.wr_fract >= 1,
- "Write fraction cannot be less than one");
+ p.wr_fract = atoi_positive(optarg);
break;
case 'g':
dirty_log_manual_caps = 0;
@@ -427,7 +425,7 @@ int main(int argc, char *argv[])
help(argv[0]);
break;
case 'i':
- p.iterations = atoi_paranoid(optarg);
+ p.iterations = atoi_positive(optarg);
break;
case 'm':
guest_modes_cmdline(optarg);
@@ -445,12 +443,12 @@ int main(int argc, char *argv[])
p.backing_src = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive(optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 'x':
- p.slots = atoi_paranoid(optarg);
+ p.slots = atoi_positive(optarg);
break;
default:
help(argv[0]);
@@ -153,5 +153,7 @@ static inline void *align_ptr_up(void *x, size_t size)
}
int atoi_paranoid(const char *num_str);
+uint32_t atoi_positive(const char *num_str);
+uint32_t atoi_non_negative(const char *num_str);
#endif /* SELFTEST_KVM_TEST_UTIL_H */
@@ -461,8 +461,8 @@ int main(int argc, char *argv[])
p.test_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive(optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 's':
@@ -353,3 +353,19 @@ int atoi_paranoid(const char *num_str)
return num;
}
+
+uint32_t atoi_positive(const char *num_str)
+{
+ int num = atoi_paranoid(num_str);
+
+ TEST_ASSERT(num > 0, "%s is not a positive integer.\n", num_str);
+ return num;
+}
+
+uint32_t atoi_non_negative(const char *num_str)
+{
+ int num = atoi_paranoid(num_str);
+
+ TEST_ASSERT(num >= 0, "%s is not a non-negative integer.\n", num_str);
+ return num;
+}
@@ -193,15 +193,14 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "c:h:m:s:H")) != -1) {
switch (opt) {
case 'c':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
+ nr_vcpus = atoi_positive(optarg);
break;
case 'm':
- max_mem = atoi_paranoid(optarg) * size_1gb;
+ max_mem = atoi_positive(optarg) * size_1gb;
TEST_ASSERT(max_mem > 0, "memory size must be >0");
break;
case 's':
- slot_size = atoi_paranoid(optarg) * size_1gb;
+ slot_size = atoi_positive(optarg) * size_1gb;
TEST_ASSERT(slot_size > 0, "slot size must be >0");
break;
case 'H':
@@ -166,8 +166,8 @@ int main(int argc, char *argv[])
guest_percpu_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive(optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d",
max_vcpus);
break;
@@ -175,7 +175,7 @@ int main(int argc, char *argv[])
p.partition_vcpu_memory_access = false;
break;
case 'i':
- p.nr_memslot_modifications = atoi_paranoid(optarg);
+ p.nr_memslot_modifications = atoi_positive(optarg);
break;
case 'h':
default:
@@ -892,33 +892,21 @@ static bool parse_args(int argc, char *argv[],
}
break;
case 'f':
- targs->tfirst = atoi_paranoid(optarg);
- if (targs->tfirst < 0) {
- pr_info("First test to run has to be non-negative\n");
- return false;
- }
+ targs->tfirst = atoi_non_negative(optarg);
break;
case 'e':
- targs->tlast = atoi_paranoid(optarg);
- if (targs->tlast < 0 || targs->tlast >= NTESTS) {
+ targs->tlast = atoi_non_negative(optarg);
+ if (targs->tlast >= NTESTS) {
pr_info("Last test to run has to be non-negative and less than %zu\n",
NTESTS);
return false;
}
break;
case 'l':
- targs->seconds = atoi_paranoid(optarg);
- if (targs->seconds < 0) {
- pr_info("Test length in seconds has to be non-negative\n");
- return false;
- }
+ targs->seconds = atoi_non_negative(optarg);
break;
case 'r':
- targs->runs = atoi_paranoid(optarg);
- if (targs->runs <= 0) {
- pr_info("Runs per test has to be positive\n");
- return false;
- }
+ targs->runs = atoi_positive(optarg);
break;
}
}
@@ -407,7 +407,7 @@ int main(int argc, char *argv[])
#ifdef __x86_64__
if (argc > 1)
- loops = atoi_paranoid(argv[1]);
+ loops = atoi_positive(argv[1]);
else
loops = 10;