Re: [PATCH v1 1/1] arm64: Fix strrchr() matching of null terminator

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2026 01:43, Edgar E. Iglesias wrote:
> The generic Xen strrchr() implementation returns a pointer to the string
> terminator when searching for '\0', matching the standard C semantics.
> 
> The ARM64 assembly version stopped as soon as it loaded the terminator and
> returned the previous match pointer instead.  This made strrchr("", '\0')
> return NULL.

I wonder though: Why would one pass '\0' to strrchr()? If you want to find
the end of a string, more efficient (at least in the general case) options
exist (strchr(), memchr(), strlen()).

> Compare the loaded byte against the requested character before deciding
> whether to stop at the terminator, so the terminator itself can be returned
> when it is the requested character.

Nit: "..., so a pointer to the terminator ...".

> Fixes: 42c4eb6a83 ("xen: arm64: assembly optimised mem* and str*")
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

However, the function having come from Linux, imo the patch wants to go to
Linux (ideally first, but at the very least also).

Additionally, looking at strchr() - couldn't the code here be written in a
similar way, allowing to get away with just a single branch? (Arm32's pair
of functions is also pretty similar in this regard.)

Jan

> --- a/xen/arch/arm/arm64/lib/strrchr.S
> +++ b/xen/arch/arm/arm64/lib/strrchr.S
> @@ -30,11 +30,10 @@ FUNC(strrchr)
>       mov     x3, #0
>       and     w1, w1, #0xff
>  1:   ldrb    w2, [x0], #1
> -     cbz     w2, 2f
>       cmp     w2, w1
> -     b.ne    1b
> +     b.ne    2f
>       sub     x3, x0, #1
> -     b       1b
> -2:   mov     x0, x3
> +2:   cbnz    w2, 1b
> +     mov     x0, x3
>       ret
>  END(strrchr)