Re: match_token weird behavior

[Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]

On 09/17/14 13:33, Randy Dunlap wrote:
> On 09/17/14 11:20, Steve French wrote:
>> Noticing something very strange with match_token.   I had five strings
>> I need to compare a version string (protocol dialect eg. "2.1" or
>> "3.0") against, to find which it matches (if any), but adding one to
>> the list (now checking for one of six strings instead of five) causes
>> the error case to always default to element 3 (in my example looks as
>> if it matched to the 2.1 string) instead of the error case.
>>
>> enum smb_version {
>>     Smb_1 = 1,
>>     Smb_20,
>>     Smb_21,
>>     Smb_30,
>>     Smb_302,
>> };
>>
>> static const match_table_t cifs_smb_version_tokens = {
>>     { Smb_1, SMB1_VERSION_STRING },
>>     { Smb_20, SMB20_VERSION_STRING},
>>     { Smb_21, SMB21_VERSION_STRING },
>>     { Smb_30, SMB30_VERSION_STRING },
>>     { Smb_302, SMB302_VERSION_STRING },
>> };
>>
> 
> You don't tell us what the actual string values are, but I'm guessing that
> SMB302_VERSION_STRING is a subset (same in first N characters) of SMB30_VERSION_STRING. ??
> 
> In that case I think that match_token() will return a ptr to SMB_30 instead of to
> SMB_302 when the input is "3.02" (matches "3.0" due to the kernel's implementation
> of strcmp() stopping at the end of string1 (where string1 is "3.0" in this case).

Oops, it seems that I got the strcmp() parameters reversed.  Sorry about that.
Feel free to disregard my ramblings.

> 
> If that is all correct, then could your return value be off by 1?
> 
>>
>> When I add one entry to the lists above (going from 5 to 6 elements),
>> and then add one additional case for it to the switch statement, an
>> attempt to provide an unrecognized string (e.g. if I specify an illegal
>> dialect string like "9" instead of "3.0" or "2.1" etc) will now match the
>> third element (Smb_21) instead of "default" in the code snippet below.
>> Is match_token broken? Can match token only handle tables with 5
>> elements or fewer? Is there a replacement for it for this kind of thing
>> (matching a string versus which from among a list of valid strings)
>> other than match_token?  Is match_token just broken?
>>
>>     substring_t args[MAX_OPT_ARGS];
>>
>>     switch (match_token(value, cifs_smb_version_tokens, args)) {
>>     case Smb_1:
>>         vol->ops = &smb1_operations;
>>         vol->vals = &smb1_values;
>>         break;
>>     case Smb_20:
>>         vol->ops = &smb20_operations;
>>         vol->vals = &smb20_values;
>>         break;
>>     case Smb_21:
>>         vol->ops = &smb21_operations;
>>         vol->vals = &smb21_values;
>>         break;
>>     case Smb_30:
>>         vol->ops = &smb30_operations;
>>         vol->vals = &smb30_values;
>>         break;
>>     case Smb_302:
>>         vol->ops = &smb30_operations; /* currently identical with 3.0 */
>>         vol->vals = &smb302_values;
>>         break;
>>     default:
>>         cifs_dbg(VFS, "Unknown vers= option specified: %s\n", value);
>>         return 1;
>>
> 
> 


-- 
~Randy