U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: Simon Glass <sjg@chromium.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH 3/6] test: Add a test for strim()
Date: Wed, 19 Mar 2025 08:24:38 -0600	[thread overview]
Message-ID: <20250319142438.GF2640854@bill-the-cat> (raw)
In-Reply-To: <20250319115911.2204045-4-sjg@chromium.org>

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

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@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@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\x7f 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@linux.vnet.ibm.com>
    Cc: Andre Goddard Rosa <andre.goddard@gmail.com>
    Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
    Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

  reply	other threads:[~2025-03-19 14:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-19 11:59 [PATCH 0/6] Test improvements Simon Glass
2025-03-19 11:59 ` [PATCH 1/6] abuf: Add a function to copy a buffer Simon Glass
2025-03-19 11:59 ` [PATCH 2/6] abuf: Add a way to printf() into " Simon Glass
2025-03-19 11:59 ` [PATCH 3/6] test: Add a test for strim() Simon Glass
2025-03-19 14:24   ` Tom Rini [this message]
2025-03-19 15:04     ` Simon Glass
2025-03-19 15:38       ` Tom Rini
2025-03-20  3:43         ` Simon Glass
2025-03-20 14:15           ` Tom Rini
2025-04-02 19:22             ` Simon Glass
2025-03-19 11:59 ` [PATCH 4/6] membuf: Add an easy way to set up a buffer with data Simon Glass
2025-03-19 11:59 ` [PATCH 5/6] sandbox: Enable PHYS_64BIT for 64-bit builds Simon Glass
2025-03-19 11:59 ` [PATCH 6/6] lib: Provide a signed version of simple_itoa() Simon Glass
2025-03-19 12:12   ` Heinrich Schuchardt
2025-03-19 14:20     ` Tom Rini
2025-03-19 15:04       ` Simon Glass
2025-03-19 15:36         ` Tom Rini
2025-03-20  3:43           ` Simon Glass
2025-03-30 14:45             ` Tom Rini
2025-03-19 15:04     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250319142438.GF2640854@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox