On Wed, Mar 19, 2025 at 03:04:16PM +0000, Simon Glass wrote:
> 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.Did your test add a testcase for the situation described above? -- Tom
signature.asc
Description: PGP signature
