[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

From: Connor Kuehl
Subject: Re: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()
Date: Thu, 1 Apr 2021 13:09:25 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0

On 4/1/21 12:24 PM, Max Reitz wrote:
On 01.04.21 17:52, Connor Kuehl wrote:
That's qemu_rbd_unescape()'s job! No need to duplicate the labor.

Furthermore, this was causing some confusion in the parsing logic to
where the caller might test for the presence of a character to split on
like so:

if (strchr(image_name, '/')) {
         found_str = qemu_rbd_next_tok(image_name, '/', &image_name);

When qemu_rbd_next_tok() performs unescaping as a side effect, the
parser can get confused thinking that it can split this string, but
really the delimiter '/' gets unescaped and so qemu_rbd_next_tok() never
"finds" the delimiter and consumes the rest of the token input stream.

I don’t fully understand.  I understand the strchr() problem and the thing you explain next.  But I would have thought that’s a problem of using strchr(), i.e. that we’d need a custom function to do strchr() but consider escaped characters.  If it’s just about true/false like in your example, it could be a new version of qemu_rbd_next_tok() that does not modify the string.

I went back and forth a lot on the different ways this can be fixed before sending this, and I agree the most consistent fix here would be to add an escape-aware strchr. Initially, adding another libc-like function with more side effects wasn't as appealing to me as removing the side effects entirely to separate mechanism vs. policy. Your example below convinced me that this patch would split the token in unexpected ways. I'll send a v2 :-)



Should it?  I would have fully expected it to not be split and the parser complains that the input is invalid.  Or, let’s say, "foo\/bar/baz” should be split into “foo\/bar” and “baz”.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]