[v2,3/5] lib/cmdline: Allow get_options() to take 0 to validate the input

Message ID 20210120214547.89770-3-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • [v2,1/5] lib/cmdline_kunit: add a new test case for get_options()
Related show

Commit Message

Andy Shevchenko Jan. 20, 2021, 9:45 p.m.
Allow get_options() to take 0 as a number of integers parameter to validate
the input.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/cmdline.c       | 14 +++++++++++---
 lib/cmdline_kunit.c | 10 +++++++++-
 2 files changed, 20 insertions(+), 4 deletions(-)

Comments

Bartosz Golaszewski Jan. 22, 2021, 11:15 a.m. | #1
On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>

> Allow get_options() to take 0 as a number of integers parameter to validate

> the input.

>

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> ---

>  lib/cmdline.c       | 14 +++++++++++---

>  lib/cmdline_kunit.c | 10 +++++++++-

>  2 files changed, 20 insertions(+), 4 deletions(-)

>

> diff --git a/lib/cmdline.c b/lib/cmdline.c

> index 2a9ae2143e42..1106a8bcd63e 100644

> --- a/lib/cmdline.c

> +++ b/lib/cmdline.c

> @@ -91,6 +91,9 @@ EXPORT_SYMBOL(get_option);

>   *     full, or when no more numbers can be retrieved from the

>   *     string.

>   *

> + *     When @nints is 0, the function just validates the given @str and

> + *     returns amount of parseable integers as described below.


I'm not a native English speaker but it sounds like this should be
"returns the amount".

> + *

>   *     Returns:

>   *

>   *     The first element is filled by the amount of the collected numbers

> @@ -103,15 +106,20 @@ EXPORT_SYMBOL(get_option);

>

>  char *get_options(const char *str, int nints, int *ints)

>  {

> +       bool validate = nints == 0;


bool validate = (nints == 0) would be clearer IMO.

>         int res, i = 1;

>

> -       while (i < nints) {

> -               res = get_option((char **)&str, ints + i);

> +       while (i < nints || validate) {

> +               int *pint = validate ? ints : ints + i;

> +

> +               res = get_option((char **)&str, pint);

>                 if (res == 0)

>                         break;

>                 if (res == 3) {

> +                       int n = validate ? 0 : nints - i;

>                         int range_nums;

> -                       range_nums = get_range((char **)&str, ints + i, nints - i);

> +

> +                       range_nums = get_range((char **)&str, pint, n);

>                         if (range_nums < 0)

>                                 break;

>                         /*

> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c

> index 74da9ed61779..a6119c164b48 100644

> --- a/lib/cmdline_kunit.c

> +++ b/lib/cmdline_kunit.c

> @@ -109,15 +109,23 @@ static void cmdline_do_one_range_test(struct kunit *test, const char *in,

>  {

>         unsigned int i;

>         int r[16];

> +       int *p;

>

>  #define FMT    "in test %u"

>  #define FMT2   "expected %d numbers, got %d"

>  #define FMT3   "at %d"

>         memset(r, 0, sizeof(r));

>         get_options(in, ARRAY_SIZE(r), r);

> -       KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " " FMT2, n, e[0], r[0]);

> +       KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " (parsed) " FMT2, n, e[0], r[0]);

>         for (i = 1; i < ARRAY_SIZE(r); i++)

>                 KUNIT_EXPECT_EQ_MSG(test, r[i], e[i], FMT " " FMT3, n, i);

> +

> +       memset(r, 0, sizeof(r));

> +       get_options(in, 0, r);

> +       KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " (validated) " FMT2, n, e[0], r[0]);

> +

> +       p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));

> +       KUNIT_EXPECT_PTR_EQ_MSG(test, p, (int *)0, FMT " out of bound " FMT3, n, p - r);

>  #undef FMT3

>  #undef FMT2

>  #undef FMT


Same as the other patch - just put the formatting strings into the messages.

Bart

> --

> 2.29.2

>
Andy Shevchenko Jan. 22, 2021, 12:13 p.m. | #2
On Fri, Jan 22, 2021 at 12:15:20PM +0100, Bartosz Golaszewski wrote:
> On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko

> <andriy.shevchenko@linux.intel.com> wrote:


...

> > + *     When @nints is 0, the function just validates the given @str and

> > + *     returns amount of parseable integers as described below.

> 

> I'm not a native English speaker but it sounds like this should be

> "returns the amount".


Sounds reasonable. Fixed for v3.

...

> > +       bool validate = nints == 0;

> 

> bool validate = (nints == 0) would be clearer IMO.


I don't see the benefit, but I have changed.

...

> Same as the other patch - just put the formatting strings into the messages.


Okay, I changed.


Thanks for review!

-- 
With Best Regards,
Andy Shevchenko

Patch

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 2a9ae2143e42..1106a8bcd63e 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -91,6 +91,9 @@  EXPORT_SYMBOL(get_option);
  *	full, or when no more numbers can be retrieved from the
  *	string.
  *
+ *	When @nints is 0, the function just validates the given @str and
+ *	returns amount of parseable integers as described below.
+ *
  *	Returns:
  *
  *	The first element is filled by the amount of the collected numbers
@@ -103,15 +106,20 @@  EXPORT_SYMBOL(get_option);
 
 char *get_options(const char *str, int nints, int *ints)
 {
+	bool validate = nints == 0;
 	int res, i = 1;
 
-	while (i < nints) {
-		res = get_option((char **)&str, ints + i);
+	while (i < nints || validate) {
+		int *pint = validate ? ints : ints + i;
+
+		res = get_option((char **)&str, pint);
 		if (res == 0)
 			break;
 		if (res == 3) {
+			int n = validate ? 0 : nints - i;
 			int range_nums;
-			range_nums = get_range((char **)&str, ints + i, nints - i);
+
+			range_nums = get_range((char **)&str, pint, n);
 			if (range_nums < 0)
 				break;
 			/*
diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
index 74da9ed61779..a6119c164b48 100644
--- a/lib/cmdline_kunit.c
+++ b/lib/cmdline_kunit.c
@@ -109,15 +109,23 @@  static void cmdline_do_one_range_test(struct kunit *test, const char *in,
 {
 	unsigned int i;
 	int r[16];
+	int *p;
 
 #define FMT	"in test %u"
 #define FMT2	"expected %d numbers, got %d"
 #define FMT3	"at %d"
 	memset(r, 0, sizeof(r));
 	get_options(in, ARRAY_SIZE(r), r);
-	KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " " FMT2, n, e[0], r[0]);
+	KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " (parsed) " FMT2, n, e[0], r[0]);
 	for (i = 1; i < ARRAY_SIZE(r); i++)
 		KUNIT_EXPECT_EQ_MSG(test, r[i], e[i], FMT " " FMT3, n, i);
+
+	memset(r, 0, sizeof(r));
+	get_options(in, 0, r);
+	KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], FMT " (validated) " FMT2, n, e[0], r[0]);
+
+	p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));
+	KUNIT_EXPECT_PTR_EQ_MSG(test, p, (int *)0, FMT " out of bound " FMT3, n, p - r);
 #undef FMT3
 #undef FMT2
 #undef FMT