RE: [PATCH] s390: Fix strrchr() implementation

[Roberto Sassu]

> From: Heiko Carstens [mailto:hca@xxxxxxxxxxxxx]
> Sent: Tuesday, October 5, 2021 1:13 PM
> On Tue, Oct 05, 2021 at 09:26:21AM +0200, Roberto Sassu wrote:
> > Access the string at len - 1 instead of len.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > ---
> >  arch/s390/lib/string.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/s390/lib/string.c b/arch/s390/lib/string.c
> > index cfcdf76d6a95..162a391788ad 100644
> > --- a/arch/s390/lib/string.c
> > +++ b/arch/s390/lib/string.c
> > @@ -261,12 +261,12 @@ char *strrchr(const char *s, int c)
> >  {
> >         size_t len = __strend(s) - s;
> >
> > -       if (len)
> > -	       do {
> > -		       if (s[len] == (char) c)
> > -			       return (char *) s + len;
> > -	       } while (--len > 0);
> > -       return NULL;
> > +	if (len)
> > +		do {
> > +			if (s[len - 1] == (char) c)
> > +				return (char *) s + len - 1;
> > +		} while (--len > 0);
> > +	return NULL;
> 
> You missed to tell what this is supposed to fix. The patch however is
> incorrect: the terminating null byte is considered part of the
> string. With your patch strrchr(somestring, 0) would not work
> correctly anymore.

Hi Heiko

yes, sorry. I didn't consider that.

> However our strrchr implementation is indeed broken, since for an
> empty string and searching for the null byte would incorrectly return
> NULL. Luckily there is not a single invocation in the kernel which
> doing that.

Ok. However, the main reason of this patch is that s[0] is not
evaluated, when len > 0. I will provide a new version of the patch.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua