Re: [PATCH v3 04/31] lib: Add a way to find the postiion of a trailing number

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Jan 19, 2022 at 12:27:09PM +0100, Heinrich Schuchardt wrote:
> On 1/19/22 02:42, Simon Glass wrote:
> > At present it is not possible to find out which part of the string is the
> > number part and which is before it. Add a new variant which provides this
> > feature, so we can separate the two in the caller.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> > Changes in v3:
> > - Change the function to return a pointer to the first digit
> > - Add some tests, including one for 'abc123def456'
> > 
> >   include/vsprintf.h | 18 ++++++++++++++++++
> >   lib/strto.c        | 14 ++++++++++++--
> >   test/str_ut.c      | 13 ++++++++++++-
> >   3 files changed, 42 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/vsprintf.h b/include/vsprintf.h
> > index 01d2248e04d..ce7a7aaa1cc 100644
> > --- a/include/vsprintf.h
> > +++ b/include/vsprintf.h
> > @@ -118,6 +118,24 @@ long trailing_strtol(const char *str);
> >    */
> >   long trailing_strtoln(const char *str, const char *end);
> > 
> > +/**
> > + * trailing_strtoln_end() - extract trailing integer from a fixed-length string
> > + *
> > + * Given a fixed-length string this finds a trailing number on the string
> > + * and returns it. For example, "abc123" would return 123. Only the
> > + * characters between @str and @end - 1 are examined. If @end is NULL, it is
> > + * set to str + strlen(str).
> > + *
> > + * @str:	String to exxamine
> > + * @end:	Pointer to end of string to examine, or NULL to use the
> > + *		whole string
> > + * @endp:	If non-NULL, this is set to point to the character where the
> > + *	number starts, e.g. for "mmc0" this would be point to the '0'; if no
> > + *	trailing number is found, it is set to the end of the string
> > + * @return training number if found, else -1
> 
> Return:
> 
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

One of other common mistakes(?) that I can see in the repository is
a violation of the rule below:
===8<===
Function parameters
~~~~~~~~~~~~~~~~~~~

Each function argument should be described in order, immediately following
                                                     ^^^^^^^^^^^^^^^^^^^^^
the short function description.  Do not leave a blank line between the
function description and the arguments, nor between the arguments.
===>8===

For instance, in this patch,
> > +/**
> > + * trailing_strtoln_end() - extract trailing integer from a fixed-length string
> > + *
> > + * Given a fixed-length string this finds a trailing number on the string
> > + * and returns it. For example, "abc123" would return 123. Only the
> > + * characters between @str and @end - 1 are examined. If @end is NULL, it is
> > + * set to str + strlen(str).
> > + *
> > + * @str:	String to exxamine
(snip)

The structure of trailing_strtoln_end() looks like:
  <short function description>
  <blank line>
  <long function description>
  <blank line>
  <function arguments>

Doesn't this matter so far in formatting Sphinx texts?

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > + */
> > +long trailing_strtoln_end(const char *str, const char *end, char const **endp);
> > +
> >   /**
> >    * panic() - Print a message and reset/hang
> >    *
> > diff --git a/lib/strto.c b/lib/strto.c
> > index 58fc10ecaea..72cfef660fc 100644
> > --- a/lib/strto.c
> > +++ b/lib/strto.c
> > @@ -183,7 +183,7 @@ long long simple_strtoll(const char *cp, char **endp, unsigned int base)
> >   	return simple_strtoull(cp, endp, base);
> >   }
> > 
> > -long trailing_strtoln(const char *str, const char *end)
> > +long trailing_strtoln_end(const char *str, const char *end, char const **endp)
> >   {
> >   	const char *p;
> > 
> > @@ -192,14 +192,24 @@ long trailing_strtoln(const char *str, const char *end)
> >   	p = end - 1;
> >   	if (p > str && isdigit(*p)) {
> >   		do {
> > -			if (!isdigit(p[-1]))
> > +			if (!isdigit(p[-1])) {
> > +				if (endp)
> > +					*endp = p;
> >   				return dectoul(p, NULL);
> > +			}
> >   		} while (--p > str);
> >   	}
> > +	if (endp)
> > +		*endp = end;
> > 
> >   	return -1;
> >   }
> > 
> > +long trailing_strtoln(const char *str, const char *end)
> > +{
> > +	return trailing_strtoln_end(str, end, NULL);
> > +}
> > +
> >   long trailing_strtol(const char *str)
> >   {
> >   	return trailing_strtoln(str, NULL);
> > diff --git a/test/str_ut.c b/test/str_ut.c
> > index 058b3594379..5a844347c2b 100644
> > --- a/test/str_ut.c
> > +++ b/test/str_ut.c
> > @@ -244,7 +244,9 @@ STR_TEST(str_xtoa, 0);
> > 
> >   static int str_trailing(struct unit_test_state *uts)
> >   {
> > -	char str1[] = "abc123def";
> > +	const char str1[] = "abc123def";
> > +	const char str2[] = "abc123def456";
> > +	const char *end;
> > 
> >   	ut_asserteq(-1, trailing_strtol(""));
> >   	ut_asserteq(-1, trailing_strtol("123"));
> > @@ -259,6 +261,15 @@ static int str_trailing(struct unit_test_state *uts)
> > 
> >   	ut_asserteq(3, trailing_strtol("a3"));
> > 
> > +	ut_asserteq(123, trailing_strtoln_end(str1, str1 + 6, &end));
> > +	ut_asserteq(3, end - str1);
> > +
> > +	ut_asserteq(-1, trailing_strtoln_end(str1, str1 + 7, &end));
> > +	ut_asserteq(7, end - str1);
> > +
> > +	ut_asserteq(456, trailing_strtoln_end(str2, NULL, &end));
> > +	ut_asserteq(9, end - str2);
> > +
> >   	return 0;
> >   }
> >   STR_TEST(str_trailing, 0);
>