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

[Connor Kuehl <ckuehl@redhat.com>]

On 4/6/21 9:24 AM, Max Reitz wrote:
> On 01.04.21 23:01, Connor Kuehl wrote:
>> [..]
>> 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.

Do you have a strong preference for this? I agree that 
qemu_rbd_next_tok() could grow this functionality, but I think it'd be 
simpler to keep it separate in the form of qemu_rbd_strchr().

> 
> 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.

Aha, good catch. I'll fix this up.

Thank you,

Connor