[PATCH 3/6] test: Add a test for strim()

[Simon Glass]

Hi Tom,

On Wed, 19 Mar 2025 at 15:24, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Mar 19, 2025 at 12:59:05PM +0100, Simon Glass wrote:
>
> > This function trims whitespace from the start and end of a string. Add a
> > test for it.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  test/lib/string.c | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
>
> This got me to ask "What even is using strim and where did it come
> from?". To which the answer is:
> - A few places, but it's probably reasonable.
> - Linux, pre-2011.
>
> I say the latter because we're missing a bug fix to the strim function
> that's been there since 2011:
>
> commit 66f6958e69d8055277356d3cc2e7a1d734db1755
> Author: Michael Holzheu <holzheu at linux.vnet.ibm.com>
> Date:   Mon Oct 31 17:12:37 2011 -0700
>
>     lib/string.c: fix strim() semantics for strings that have only blanks
>
>     Commit 84c95c9acf0 ("string: on strstrip(), first remove leading spaces
>     before running over str") improved  the performance of the strim()
>     function.
>
>     Unfortunately this changed the semantics of strim() and broke my code.
>     Before the patch it was possible to use strim() without using the return
>     value for removing trailing spaces from strings that had either only
>     blanks or only trailing blanks.
>
>     Now this does not work any longer for strings that *only* have blanks.
>
>     Before patch: "   " -> ""    (empty string)
>     After patch:  "   " -> "   " (no change)
>
>     I think we should remove your patch to restore the old behavior.
>
>     The description (lib/string.c):
>
>      * Note that the first trailing whitespace is replaced with a %NUL-terminator
>
>     => The first trailing whitespace of a string that only has whitespace
>        characters is the first whitespace
>
>     The patch restores the old strim() semantics.
>
>     Signed-off-by: Michael Holzheu <holzheu at linux.vnet.ibm.com>
>     Cc: Andre Goddard Rosa <andre.goddard at gmail.com>
>     Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
>     Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
>     Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
>     Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
>
> --
> Tom

Here is the comment for the function, of note given Linux's disdain
for commenting code:

/**
 * strim - Removes leading and trailing whitespace from @s.
 * @s: The string to be stripped.
 *
 * Note that the first trailing whitespace is replaced with a %NUL-terminator
 * in the given string @s. Returns a pointer to the first non-whitespace
 * character in @s.
 */

Given that comment, I don't see a bug here. But of course we could add
a test for it and adjust the function too. PLMK.

Regards,
Simon