Message ID | 20220119014315.1938157-5-sjg@chromium.org |
---|---|
State | RFC |
Delegated to: | Tom Rini |
Headers | show |
Series | Initial implementation of standard boot | expand |
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 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);
Hi Heinrich, On Wed, 19 Jan 2022 at 04:33, Heinrich Schuchardt <xypron.glpk@gmx.de> 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 Yes I am slowly getting used to this. Is there any way to accept both? U=Boot has used @return for years so there is lots of itin the code. Regards, Simon
On Wed, Jan 19, 2022 at 07:37:23AM -0700, Simon Glass wrote: > Hi Heinrich, > > On Wed, 19 Jan 2022 at 04:33, Heinrich Schuchardt <xypron.glpk@gmx.de> 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 > > Yes I am slowly getting used to this. > > Is there any way to accept both? U=Boot has used @return for years so > there is lots of itin the code. Can we just do a coccinelle script to just update the codebase?
On 1/19/22 16:11, Tom Rini wrote: > On Wed, Jan 19, 2022 at 07:37:23AM -0700, Simon Glass wrote: >> Hi Heinrich, >> >> On Wed, 19 Jan 2022 at 04:33, Heinrich Schuchardt <xypron.glpk@gmx.de> 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 >> >> Yes I am slowly getting used to this. >> >> Is there any way to accept both? U=Boot has used @return for years so >> there is lots of itin the code. This was before we started publishing HTML documentation on readthedocs.io. Sphinx will not build with @return or @return:. > > Can we just do a coccinelle script to just update the codebase? > For new lines we just need to remind the author. For existing code sed should be good enough, e.g. find . -name '*.c' -exec \ sed -i 's/^\(\s\)\*\(\s*\)@return\(\s\)/\1*\2Return:\3/' {} \; find . -name '*.h' -exec \ sed -i 's/^\(\s\)\*\(\s*\)@return\(\s\)/\1*\2Return:\3/' {} \; I just sent a patch. Best regards Heinrich
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); >
Hi Takahiro, On Wed, 19 Jan 2022 at 20:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > 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? It doesn't seem to, luckily. I'm not too impressed with how simplistic Sphinx is with formatting, but it seems OK in that case. Regards, Simon
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 + */ +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);
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(-)