[Spice-devel] [usbredir PATCH 6/6] usbredirfilter: fix filter parsing for windows (mingw)

[Hans de Goede]

Hi,

On 05/03/2012 06:09 PM, Christophe Fergeau wrote:
> On Thu, May 03, 2012 at 06:04:39PM +0300, Uri Lublin wrote:
>> From: Arnon Gilboa<agilboa at redhat.com>
>>
>> no strtok_r (reentrent) available, so use strtok instead.
>
> Wouldn't it be safer to use strtok_s? Any application using threads and
> strtok and libusbredirparser could get issues with this change no?

I have to agree with Christophe here, I would really like to keep
this code safe for calling from multiple threads, even if that
means some #ifdef stuff.

>> Modified-by: Uri Lublin<uril at redhat.com>
>> ---
>>   usbredirparser/usbredirfilter.c |   16 +++++++++++-----
>>   1 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/usbredirparser/usbredirfilter.c b/usbredirparser/usbredirfilter.c
>> index b74c921..a1213cc 100644
>> --- a/usbredirparser/usbredirfilter.c
>> +++ b/usbredirparser/usbredirfilter.c
>> @@ -29,7 +29,7 @@ int usbredirfilter_string_to_rules(
>>       const char *filter_str, const char *token_sep, const char *rule_sep,
>>       struct usbredirfilter_rule **rules_ret, int *rules_count_ret)
>>   {
>> -    char *rule, *rule_saveptr, *token, *token_saveptr, *ep;
>> +    char *rule, *token, *ep, *buf_end, *rule_end;
>>       struct usbredirfilter_rule *rules = NULL;
>>       int i, rules_count, *values, ret = 0;
>>       char *buf = NULL;
>> @@ -63,17 +63,19 @@ int usbredirfilter_string_to_rules(
>>       }
>>
>>       /* And actually parse the string */
>> +    buf_end = buf + strlen(buf);
>>       rules_count = 0;
>> -    rule = strtok_r(buf, rule_sep,&rule_saveptr);
>> +    rule = strtok(buf, rule_sep);
>>       while (rule) {
>> +        rule_end = rule + strlen(rule);
>>           /* We treat the filter rule as an array of ints for easier parsing */
>>           values = (int *)&rules[rules_count];
>> -        token = strtok_r(rule, token_sep,&token_saveptr);
>> +        token = strtok(rule, token_sep);
>>           for (i = 0; i<  5&&  token; i++) {
>>               values[i] = strtol(token,&ep, 0);
>>               if (*ep)
>>                   break;
>> -            token = strtok_r(NULL, token_sep,&token_saveptr);
>> +            token = strtok(NULL, token_sep);
>>           }
>>           if (i != 5 || token != NULL ||
>>                   usbredirfilter_verify(&rules[rules_count], 1)) {
>> @@ -81,7 +83,11 @@ int usbredirfilter_string_to_rules(
>>               goto leave;
>>           }
>>           rules_count++;
>> -        rule = strtok_r(NULL, rule_sep,&rule_saveptr);
>> +        if (rule_end<  buf_end) {
>> +            rule = strtok(rule_end + 1, rule_sep);
>> +        } else {
>> +            rule = NULL;
>> +        }
>
> rule = strtok(NULL, rule_sep); didn't work there? I'm bad with
> strtok/strtok_r, but by reading the manpage, it doesn't seem things should
> be handled differently there.

Again I agree with Christophe :)

All the other patches are ok. I don't know if you've noticed it yet,
but usbredir has an official git repo here now:
http://cgit.freedesktop.org/spice/usbredir/

Feel free to push the other patches there (since it is part of the
spice group git repos you should have commit access).

Regards,

Hans