| 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() | expand |
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
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
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
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
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>
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 >
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>
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), {} };
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(+)