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

[Stefano Garzarella]

On Fri, Apr 09, 2021 at 09:38:54AM -0500, 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>
> ---
>  v2 -> v3:
>    * Update qemu_rbd_strchr to only skip if there's a delimiter AND the
>      next character is not the NUL terminator
>
> 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..291e3f09e1 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[1] != '\0') {
> +            ++p;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +

IIUC this is similar to the code used in qemu_rbd_next_tok().
To avoid code duplication can we use this new function inside it?

I mean something like this (not tested):

diff --git a/block/rbd.c b/block/rbd.c
index f098a89c7b..eb6a839362 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -119,15 +119,8 @@ static char *qemu_rbd_next_tok(char *src, char delim, char 
**p)
 
     *p = NULL;
 
-    for (end = src; *end; ++end) {
-        if (*end == delim) {
-            break;
-        }
-        if (*end == '\\' && end[1] != '\0') {
-            end++;
-        }
-    }
-    if (*end == delim) {
+    end = qemu_rbd_strchr(src, delim);
+    if (end && *end == delim) {
         *p = end + 1;
         *end = '\0';
     }


The rest LGTM!

Thanks for fixing this issue,
Stefano