[PATCH 0/2] minor lib/cmdline enhancements

[PATCH 0/2] minor lib/cmdline enhancements

[Dmitry Antipov]

Add KUnit-based test for 'memparse()' and fix
kernel-doc glitches found in lib/cmdline.c.

Dmitry Antipov (2):
  lib/cmdline_kunit: add test case for memparse()
  lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

 lib/cmdline.c             |  7 ++++--
 lib/tests/cmdline_kunit.c | 50 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

-- 
2.52.0

[PATCH 1/2] lib/cmdline_kunit: add test case for memparse()

[Dmitry Antipov]

Better late than never, now there is a long-awaited basic
test for 'memparse()' which is provided by cmdline.c.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 lib/tests/cmdline_kunit.c | 50 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
index c1602f797637..afde35f44690 100644
--- a/lib/tests/cmdline_kunit.c
+++ b/lib/tests/cmdline_kunit.c
@@ -139,11 +139,61 @@ static void cmdline_test_range(struct kunit *test)
 	} while (++i < ARRAY_SIZE(cmdline_test_range_strings));
 }
 
+struct cmdline_test_memparse_entry {
+	const char *input;
+	const char *unrecognized;
+	unsigned long long result;
+};
+
+static const struct cmdline_test_memparse_entry testdata[] = {
+	{ "0",			"",	0ULL },
+	{ "1",			"",	1ULL },
+	{ "a",			"a",	0ULL },
+	{ "0xb",		"",	11ULL },
+	{ "0xz",		"x",	0ULL },
+	{ "1234",		"",	1234ULL },
+	{ "04567",		"",	2423ULL },
+	{ "0x9876",		"",	39030LL },
+	{ "05678",		"8",	375ULL },
+	{ "0xabcdefz",		"z",	11259375ULL },
+	{ "0cdba",		"c",	0ULL },
+	{ "4K",			"",	4096ULL },
+	{ "0x10k@0xaaaabbbb",	"@",	16384ULL },
+	{ "32M",		"",	33554432ULL },
+	{ "067m:foo",		":",	57671680ULL },
+	{ "2G;bar=baz",		";",	2147483648ULL },
+	{ "07gz",		"z",	7516192768ULL },
+	{ "3T+data",		"+",	3298534883328ULL },
+	{ "04t,ro",		",",	4398046511104ULL },
+	{ "012p",		"",	11258999068426240ULL },
+	{ "7P,sync",		",",	7881299347898368ULL },
+	{ "0x2e",		"",	46ULL },
+	{ "2E and more",	" ",	2305843009213693952ULL },
+	{ "18446744073709551615", "",	18446744073709551615ULL },
+	{ "18446744073709551616", "",	0ULL }
+};
+
+static void cmdline_test_memparse(struct kunit *test)
+{
+	const struct cmdline_test_memparse_entry *e;
+	unsigned long long ret;
+	char *retptr;
+
+	for (e = testdata; e < testdata + ARRAY_SIZE(testdata); e++) {
+		ret = memparse(e->input, &retptr);
+		KUNIT_EXPECT_EQ_MSG(test, ret, e->result,
+				    "    when parsing '%s'", e->input);
+		KUNIT_EXPECT_EQ_MSG(test, *retptr, *e->unrecognized,
+				    "    when parsing '%s'", e->input);
+	}
+}
+
 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),
+	KUNIT_CASE(cmdline_test_memparse),
 	{}
 };
 
-- 
2.52.0

Re: [PATCH 1/2] lib/cmdline_kunit: add test case for memparse()

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 02:12:53PM +0300, Dmitry Antipov wrote:
> Better late than never, now there is a long-awaited basic
> test for 'memparse()' which is provided by cmdline.c.

Thank you for this! Really appreciate!

See a couple of nit-picks below.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
assuming they will be amended.

...

> +static const struct cmdline_test_memparse_entry testdata[] = {
> +	{ "0",			"",	0ULL },
> +	{ "1",			"",	1ULL },
> +	{ "a",			"a",	0ULL },
> +	{ "0xb",		"",	11ULL },
> +	{ "0xz",		"x",	0ULL },
> +	{ "1234",		"",	1234ULL },
> +	{ "04567",		"",	2423ULL },
> +	{ "0x9876",		"",	39030LL },
> +	{ "05678",		"8",	375ULL },
> +	{ "0xabcdefz",		"z",	11259375ULL },
> +	{ "0cdba",		"c",	0ULL },

> +	{ "4K",			"",	4096ULL },
> +	{ "0x10k@0xaaaabbbb",	"@",	16384ULL },
> +	{ "32M",		"",	33554432ULL },

These can utilize constants from sizes.h.

> +	{ "067m:foo",		":",	57671680ULL },

> +	{ "2G;bar=baz",		";",	2147483648ULL },

Ditto.

> +	{ "07gz",		"z",	7516192768ULL },
> +	{ "3T+data",		"+",	3298534883328ULL },

> +	{ "04t,ro",		",",	4398046511104ULL },

Ditto. (However I don't remember the maximum available there.)

> +	{ "012p",		"",	11258999068426240ULL },
> +	{ "7P,sync",		",",	7881299347898368ULL },
> +	{ "0x2e",		"",	46ULL },
> +	{ "2E and more",	" ",	2305843009213693952ULL },
> +	{ "18446744073709551615", "",	18446744073709551615ULL },

> +	{ "18446744073709551616", "",	0ULL }

Leave trailing comma as it's not a terminator.

Can we also have one with more than 20 decimal digits?

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] lib/cmdline_kunit: add test case for memparse()

[Dmitry Antipov]

On Thu, 2026-01-22 at 13:46 +0200, Andy Shevchenko wrote:

> Can we also have one with more than 20 decimal digits?

Hmmm... running simple userspace test,

strtoull("569202370375329612767", ...)

returns ULLONG_MAX (and sets errno to -ERANGE), but

memparse("569202370375329612767", ...)

seems blindly overflows to -2646695909666487329.

I'm strongly suspecting that 'memparse()' should be carefully
tweaked to use 'kstrtoull()' instead of 'simple_strtoull()'.

Dmitry

Re: [PATCH 1/2] lib/cmdline_kunit: add test case for memparse()

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 06:00:43PM +0300, Dmitry Antipov wrote:
> On Thu, 2026-01-22 at 13:46 +0200, Andy Shevchenko wrote:
> 
> > Can we also have one with more than 20 decimal digits?
> 
> Hmmm... running simple userspace test,
> 
> strtoull("569202370375329612767", ...)
> 
> returns ULLONG_MAX (and sets errno to -ERANGE), but
> 
> memparse("569202370375329612767", ...)
> 
> seems blindly overflows to -2646695909666487329.
> 
> I'm strongly suspecting that 'memparse()' should be carefully
> tweaked to use 'kstrtoull()' instead of 'simple_strtoull()'.

It will require an intermediate buffer which your series is exactly against,
right? It's better to actually create a safe wrapper on top of
simple_strtoull().

The points are the following:
If there is a decimal base, we need to count leading 0:s.
Then add a check for how many characters were actually processed.

Partially something similar is done here:
drivers/crypto/intel/qat/qat_common/qat_uclo.c:206

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/2] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

[Dmitry Antipov]

Fix 'get_option()', 'memparse()' and 'parse_option_str()' comments
to match the commonly used style as suggested by kernel-doc -Wreturn.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 lib/cmdline.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 90ed997d9570..9ac23a0aecf7 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -43,7 +43,7 @@ static int get_range(char **str, int *pint, int n)
  *	When @pint is NULL the function can be used as a validator of
  *	the current option in the string.
  *
- *	Return values:
+ *	Return:
  *	0 - no int in string
  *	1 - int found, no subsequent comma
  *	2 - int found including a subsequent comma
@@ -145,6 +145,9 @@ EXPORT_SYMBOL(get_options);
  *
  *	Parses a string into a number.  The number stored at @ptr is
  *	potentially suffixed with K, M, G, T, P, E.
+ *
+ *	Return: The value as recognized by simple_strtoull() multiplied
+ *	by the value as specified by suffix, if any.
  */
 
 unsigned long long memparse(const char *ptr, char **retptr)
@@ -198,7 +201,7 @@ EXPORT_SYMBOL(memparse);
  *	This function parses a string containing a comma-separated list of
  *	strings like a=b,c.
  *
- *	Return true if there's such option in the string, or return false.
+ *	Return: True if there's such option in the string or false otherwise.
  */
 bool parse_option_str(const char *str, const char *option)
 {
-- 
2.52.0

Re: [PATCH 2/2] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 02:12:54PM +0300, Dmitry Antipov wrote:
> Fix 'get_option()', 'memparse()' and 'parse_option_str()' comments
> to match the commonly used style as suggested by kernel-doc -Wreturn.

So, I gave you a tag for this change in the other series and it's dropped.
Why?

-- 
With Best Regards,
Andy Shevchenko