Messages in this thread | | | Date | Thu, 13 Oct 2011 16:54:31 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] Fix strim() semantics for strings that have only blanks |
| |
On Wed, 12 Oct 2011 10:31:57 +0200 Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:
> Hello Andre, > > With git commit 84c95c9acf088c99d8793d78036b67faa5d0b851 a patch from > you went upstream where you wanted to improve 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. > > >From 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
Yes, that change makes sense.
> Also strim() explicitly does not have "__must_check", in order to use it > without using the return value. >
This observation seems to have nothing to do with the patch?
I think that strim() _should_ have __must_check annotation (which means that strim() simply disappears, which is good). It is only safe to ignore the strim() return value if the caller knows that the string started with a non-space. The number of situations where this is guaranteed are faily small, I suspect. Whereas the chances of someone screwing this up are pretty high :(
| |