Hi Tom,
On Wed, 19 Mar 2025 at 15:24, Tom Rini <tr...@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 <s...@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 <holz...@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 <holz...@linux.vnet.ibm.com>
> Cc: Andre Goddard Rosa <andre.godd...@gmail.com>
> Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
> Cc: Heiko Carstens <heiko.carst...@de.ibm.com>
> Signed-off-by: Andrew Morton <a...@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torva...@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
