[v2,1/5] lib/cmdline_kunit: add a new test case for get_options()

Message ID 20210120214547.89770-1-andriy.shevchenko@linux.intel.com
State Superseded
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.
Add a test case for get_options() which is provided by cmdline.c.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/cmdline_kunit.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Linus Walleij Jan. 21, 2021, 1:17 p.m. | #1
On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> We already have a nice helper called get_options() which can be used

> to validate the input format. Replace isrange() by using it.

>

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


This is really neat.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


Yours,
Linus Walleij
Linus Walleij Jan. 21, 2021, 1:17 p.m. | #2
On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Remove trailing comma in terminator entries to avoid potential

> expanding an array behind it.

>

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


Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


Yours,
Linus Walleij
Andy Shevchenko Jan. 21, 2021, 5:09 p.m. | #3
On Thu, Jan 21, 2021 at 02:17:47PM +0100, Linus Walleij wrote:
> On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko

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

> 

> > Remove trailing comma in terminator entries to avoid potential

> > expanding an array behind it.

> >

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

> 

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


Thanks!

Bart, I would like to send this series as a part of my Intel GPIO PR or tell me
if you wish another approach.

-- 
With Best Regards,
Andy Shevchenko
Bartosz Golaszewski Jan. 22, 2021, 10:46 a.m. | #4
On Thu, Jan 21, 2021 at 6:08 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>

> On Thu, Jan 21, 2021 at 02:17:47PM +0100, Linus Walleij wrote:

> > On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko

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

> >

> > > Remove trailing comma in terminator entries to avoid potential

> > > expanding an array behind it.

> > >

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

> >

> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>

> Thanks!

>

> Bart, I would like to send this series as a part of my Intel GPIO PR or tell me

> if you wish another approach.

>

> --

> With Best Regards,

> Andy Shevchenko

>

>


No that's fine.

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

> We already have a nice helper called get_options() which can be used

> to validate the input format. Replace isrange() by using it.

>

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

> ---


Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Bartosz Golaszewski Jan. 22, 2021, 11:11 a.m. | #6
On Wed, Jan 20, 2021 at 10:45 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>

> Add a test case for get_options() which is provided by cmdline.c.

>

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

> ---

>  lib/cmdline_kunit.c | 52 +++++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 52 insertions(+)

>

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

> index 550e7a47fd24..74da9ed61779 100644

> --- a/lib/cmdline_kunit.c

> +++ b/lib/cmdline_kunit.c

> @@ -18,6 +18,26 @@ static const int cmdline_test_values[] = {

>         1, 3, 2, 1, 1, 1, 3, 1,

>  };

>

> +static_assert(ARRAY_SIZE(cmdline_test_strings) == ARRAY_SIZE(cmdline_test_values));

> +

> +static const char *cmdline_test_range_strings[] = {

> +       "-7" , "--7"  , "-1-2"    , "7--9",

> +       "7-" , "-7--9", "7-9,"    , "9-7" ,

> +       "5-a", "a-5"  , "5-8"     , ",8-5",

> +       "+,1", "-,4"  , "-3,0-1,6", "4,-" ,

> +       " +2", " -9"  , "0-1,-3,6", "- 9" ,

> +};

> +

> +static const int cmdline_test_range_values[][16] = {

> +       { 1, -7, }, { 0, -0, }, { 4, -1, 0, +1, 2, }, { 0, 7, },

> +       { 0, +7, }, { 0, -7, }, { 3, +7, 8, +9, 0, }, { 0, 9, },

> +       { 0, +5, }, { 0, -0, }, { 4, +5, 6, +7, 8, }, { 0, 0, },

> +       { 0, +0, }, { 0, -0, }, { 4, -3, 0, +1, 6, }, { 1, 4, },

> +       { 0, +0, }, { 0, -0, }, { 4, +0, 1, -3, 6, }, { 0, 0, },

> +};

> +

> +static_assert(ARRAY_SIZE(cmdline_test_range_strings) == ARRAY_SIZE(cmdline_test_range_values));

> +

>  static void cmdline_do_one_test(struct kunit *test, const char *in, int rc, int offset)

>  {

>         const char *fmt = "Pattern: %s";

> @@ -84,10 +104,42 @@ static void cmdline_test_tail_int(struct kunit *test)

>         } while (++i < ARRAY_SIZE(cmdline_test_strings));

>  }

>

> +static void cmdline_do_one_range_test(struct kunit *test, const char *in,

> +                                     unsigned int n, const int *e)

> +{

> +       unsigned int i;

> +       int r[16];

> +

> +#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]);

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

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

> +#undef FMT3

> +#undef FMT2

> +#undef FMT


IMO these FMT macros are super confusing, please just use hardcoded
strings because you're not saving any code anyway.

Bart

> +}

> +

> +static void cmdline_test_range(struct kunit *test)

> +{

> +       unsigned int i = 0;

> +

> +       do {

> +               const char *str = cmdline_test_range_strings[i];

> +               const int *e = cmdline_test_range_values[i];

> +

> +               cmdline_do_one_range_test(test, str, i, e);

> +       } while (++i < ARRAY_SIZE(cmdline_test_range_strings));

> +}

> +

>  static struct kunit_case cmdline_test_cases[] = {

>         KUNIT_CASE(cmdline_test_noint),

>         KUNIT_CASE(cmdline_test_lead_int),

>         KUNIT_CASE(cmdline_test_tail_int),

> +       KUNIT_CASE(cmdline_test_range),

>         {}

>  };

>

> --

> 2.29.2

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

> get_options() API has some tricks to optimize that may be not so obvious

> to the caller. Update documentation to reflect current behaviour.

>

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

> ---

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

>  1 file changed, 6 insertions(+), 1 deletion(-)

>

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

> index b390dd03363b..2a9ae2143e42 100644

> --- a/lib/cmdline.c

> +++ b/lib/cmdline.c

> @@ -83,7 +83,7 @@ EXPORT_SYMBOL(get_option);

>   *     get_options - Parse a string into a list of integers

>   *     @str: String to be parsed

>   *     @nints: size of integer array

> - *     @ints: integer array

> + *     @ints: integer array (must have a room for at least one element)

>   *

>   *     This function parses a string containing a comma-separated

>   *     list of integers, a hyphen-separated range of _positive_ integers,

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

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

>   *     string.

>   *

> + *     Returns:

> + *

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

> + *     in the range. The rest is what was parsed from the @str.

> + *

>   *     Return value is the character in the string which caused

>   *     the parse to end (typically a null terminator, if @str is

>   *     completely parseable).

> --

> 2.29.2

>


Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Patch

diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
index 550e7a47fd24..74da9ed61779 100644
--- a/lib/cmdline_kunit.c
+++ b/lib/cmdline_kunit.c
@@ -18,6 +18,26 @@  static const int cmdline_test_values[] = {
 	1, 3, 2, 1, 1, 1, 3, 1,
 };
 
+static_assert(ARRAY_SIZE(cmdline_test_strings) == ARRAY_SIZE(cmdline_test_values));
+
+static const char *cmdline_test_range_strings[] = {
+	"-7" , "--7"  , "-1-2"    , "7--9",
+	"7-" , "-7--9", "7-9,"    , "9-7" ,
+	"5-a", "a-5"  , "5-8"     , ",8-5",
+	"+,1", "-,4"  , "-3,0-1,6", "4,-" ,
+	" +2", " -9"  , "0-1,-3,6", "- 9" ,
+};
+
+static const int cmdline_test_range_values[][16] = {
+	{ 1, -7, }, { 0, -0, }, { 4, -1, 0, +1, 2, }, { 0, 7, },
+	{ 0, +7, }, { 0, -7, }, { 3, +7, 8, +9, 0, }, { 0, 9, },
+	{ 0, +5, }, { 0, -0, }, { 4, +5, 6, +7, 8, }, { 0, 0, },
+	{ 0, +0, }, { 0, -0, }, { 4, -3, 0, +1, 6, }, { 1, 4, },
+	{ 0, +0, }, { 0, -0, }, { 4, +0, 1, -3, 6, }, { 0, 0, },
+};
+
+static_assert(ARRAY_SIZE(cmdline_test_range_strings) == ARRAY_SIZE(cmdline_test_range_values));
+
 static void cmdline_do_one_test(struct kunit *test, const char *in, int rc, int offset)
 {
 	const char *fmt = "Pattern: %s";
@@ -84,10 +104,42 @@  static void cmdline_test_tail_int(struct kunit *test)
 	} while (++i < ARRAY_SIZE(cmdline_test_strings));
 }
 
+static void cmdline_do_one_range_test(struct kunit *test, const char *in,
+				      unsigned int n, const int *e)
+{
+	unsigned int i;
+	int r[16];
+
+#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]);
+	for (i = 1; i < ARRAY_SIZE(r); i++)
+		KUNIT_EXPECT_EQ_MSG(test, r[i], e[i], FMT " " FMT3, n, i);
+#undef FMT3
+#undef FMT2
+#undef FMT
+}
+
+static void cmdline_test_range(struct kunit *test)
+{
+	unsigned int i = 0;
+
+	do {
+		const char *str = cmdline_test_range_strings[i];
+		const int *e = cmdline_test_range_values[i];
+
+		cmdline_do_one_range_test(test, str, i, e);
+	} while (++i < ARRAY_SIZE(cmdline_test_range_strings));
+}
+
 static struct kunit_case cmdline_test_cases[] = {
 	KUNIT_CASE(cmdline_test_noint),
 	KUNIT_CASE(cmdline_test_lead_int),
 	KUNIT_CASE(cmdline_test_tail_int),
+	KUNIT_CASE(cmdline_test_range),
 	{}
 };