| Message ID | 20210122123853.75162-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | d3c7cfc6d3dc315ca58b516303ccb0ac8b005600 |
| Headers | show |
| Series | [v3,1/6] lib/cmdline_kunit: add a new test case for get_options() | expand |
Hi Andriy, On Fri, Jan 22, 2021 at 1:39 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> > Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Thanks for your patch! > --- 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) must have room > * > * 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 the number of collected integers? Yes, the lack of articles in RU can be a disadvantage... > + * 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). With the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Andy, On Fri, Jan 22, 2021 at 1:39 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: Linus Walleij <linus.walleij@linaro.org> > Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Let's hope this is never backported to stable before [PATCH 3/6].... Gr{oetje,eeting}s, Geert
On Fri, Jan 22, 2021 at 02:13:10PM +0100, Geert Uytterhoeven wrote: > On Fri, Jan 22, 2021 at 1:39 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: ... > > + while (i < nints || validate) { > > + int *pint = validate ? ints : ints + i; > > I think you can use NULL for validation, as per the documentation for > get_option(). That's what takes me a long time to realize how this machinery works and no, unfortunately we may not use NULL, we have to keep the parsed number for the further operations. Thanks for review!
diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c index 550e7a47fd24..91737f17ce51 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,37 @@ 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]; + + memset(r, 0, sizeof(r)); + get_options(in, ARRAY_SIZE(r), r); + KUNIT_EXPECT_EQ_MSG(test, r[0], e[0], "in test %u expected %d numbers, got %d", + n, e[0], r[0]); + for (i = 1; i < ARRAY_SIZE(r); i++) + KUNIT_EXPECT_EQ_MSG(test, r[i], e[i], "in test %u at %d", n, i); +} + +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), {} };
Add a test case for get_options() which is provided by cmdline.c. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v3: drop string macros (Bart) lib/cmdline_kunit.c | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)