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 :-)
Thanks,
Connor
[..]
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”.
