Re: [PATCH v2 2/2] block/rbd: Add an escape-aware strchr helper

[Max Reitz <mreitz@redhat.com>]

On 01.04.21 23:01, Connor Kuehl wrote:
> Sometimes the parser needs to further split a token it has collected
> from the token input stream. Right now, it does a cursory check to see
> if the relevant characters appear in the token to determine if it should
> break it down further.
> 
> However, qemu_rbd_next_tok() will escape characters as it removes tokens
> from the token stream and plain strchr() won't. This can make the
> initial strchr() check slightly misleading since it implies
> qemu_rbd_next_tok() will find the token and split on it, except the
> reality is that qemu_rbd_next_tok() will pass over it if it is escaped.
> 
> Use a custom strchr to avoid mixing escaped and unescaped string
> operations.
> 
> Reported-by: Han Han <hhan@redhat.com>
> Fixes: https://bugzilla.redhat.com/1873913
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>   block/rbd.c                | 20 ++++++++++++++++++--
>   tests/qemu-iotests/231     |  4 ++++
>   tests/qemu-iotests/231.out |  3 +++
>   3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index 9071a00e3f..c0e4d4a952 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -134,6 +134,22 @@ static char *qemu_rbd_next_tok(char *src, char delim, char **p)
>       return src;
>   }
>   
> +static char *qemu_rbd_strchr(char *src, char delim)
> +{
> +    char *p;
> +
> +    for (p = src; *p; ++p) {
> +        if (*p == delim) {
> +            return p;
> +        }
> +        if (*p == '\\') {
> +            ++p;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +

So I thought you could make qemu_rbd_do_next_tok() to do this.  (I 
didn’t say you should, but bear with me.)  That would be possible by 
giving it a new parameter (e.g. @find), and if that is set, return @end 
if *end == delim after the loop, and NULL otherwise.

Now, if you add wrapper functions to make it nice, there’s not much more 
difference in lines added compared to just adding a new function, but it 
does mean your function should basically be the same as 
qemu_rbd_next_tok(), except that no splitting happens, that there is no 
*p, and that @end is returned instead of @src.

So there is one difference, and that is that qemu_rbd_next_tok() has 
this condition to skip escaped characters:

     if (*end == '\\' && end[1] != '\0') {

where qemu_rbd_strchr() has only:

     if (*p == '\\') {

And I think qemu_rbd_next_tok() is right; if the string in question has 
a trailing backslash, qemu_rbd_strchr() will ignore the final NUL and 
continue searching past the end of the string.

Max