linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* match_token weird behavior
@ 2014-09-17 18:20 Steve French
       [not found] ` <CAH2r5mvTP1oGNoetuSkPdirBb1UDH6fiNCVjs_gJNKeJ1Dz5sA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2014-09-17 18:20 UTC (permalink / raw)
  To: linux-fsdevel, LKML; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

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 },
};


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;

-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: match_token weird behavior
       [not found] ` <CAH2r5mvTP1oGNoetuSkPdirBb1UDH6fiNCVjs_gJNKeJ1Dz5sA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-09-17 20:33   ` Randy Dunlap
       [not found]     ` <5419F007.9070509-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2014-09-17 20:33 UTC (permalink / raw)
  To: Steve French, linux-fsdevel, LKML
  Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Viro

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: match_token weird behavior
       [not found]     ` <5419F007.9070509-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2014-09-17 20:36       ` Randy Dunlap
       [not found]         ` <5419F0D4.9090107-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2014-09-17 20:36 UTC (permalink / raw)
  To: Steve French, linux-fsdevel, LKML
  Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Viro

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: match_token weird behavior
       [not found]         ` <5419F0D4.9090107-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2014-09-17 20:53           ` Steve French
  2014-09-17 21:05             ` Steve French
  0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2014-09-17 20:53 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: linux-fsdevel, LKML,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Viro

For additional information the strings that are being matched against are:

#define SMB1_VERSION_STRING    "1.0"
#define SMB20_VERSION_STRING    "2.0"
#define SMB21_VERSION_STRING    "2.1"
#define SMB30_VERSION_STRING    "3.0"
#define SMB302_VERSION_STRING    "3.02"
#define SMB31_VERSION_STRING    "3.1"


The matching works as expected, e,g. specifying 3.0 gets matched to
Smb_30, before and/after adding the sixth element to the match_table_t
- except that unmatched items (picking a dialect that doesn't exist
like "5.1") matches to Smb_21 where it used to fall through to the
default (error) case.

It got me a little worried that there MAX_OPT_ARGS is 3 and I am
getting the third element in the case of the error.

Looking at other examples in the kernel were strange e.g. ext4/super.c has this

        /*
         * Initialize args struct so we know whether arg was
         * found; some options take optional arguments.
         */
        args[0].to = args[0].from = NULL;
        token = match_token(p, tokens, args);
        (and then passes args down to a large helper function handle_mount_opt)

Initializing args didn't seem to help in the cifs case

On Wed, Sep 17, 2014 at 3:36 PM, Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> 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



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: match_token weird behavior
  2014-09-17 20:53           ` Steve French
@ 2014-09-17 21:05             ` Steve French
       [not found]               ` <CAH2r5muvhC6ww_U8o4mN119ptnNsuT+mcSmfaFbP7DqV0ZQpiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2014-09-17 21:05 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-fsdevel, LKML, linux-cifs@vger.kernel.org, Al Viro

Looking at more examples, some of which are much larger match tables
maybe it has to do with how the final entry is defined.  In this
example the NULL match is explicitly stated.

static const match_table_t cifs_secflavor_tokens = {
    { Opt_sec_krb5, "krb5" },
    { Opt_sec_krb5i, "krb5i" },
    { Opt_sec_krb5p, "krb5p" },
    { Opt_sec_ntlmsspi, "ntlmsspi" },
    { Opt_sec_ntlmssp, "ntlmssp" },
    { Opt_ntlm, "ntlm" },
    { Opt_sec_ntlmi, "ntlmi" },
    { Opt_sec_ntlmv2, "nontlm" },
    { Opt_sec_ntlmv2, "ntlmv2" },
    { Opt_sec_ntlmv2i, "ntlmv2i" },
    { Opt_sec_lanman, "lanman" },
    { Opt_sec_none, "none" },

    { Opt_sec_err, NULL }
};

On Wed, Sep 17, 2014 at 3:53 PM, Steve French <smfrench@gmail.com> wrote:
> For additional information the strings that are being matched against are:
>
> #define SMB1_VERSION_STRING    "1.0"
> #define SMB20_VERSION_STRING    "2.0"
> #define SMB21_VERSION_STRING    "2.1"
> #define SMB30_VERSION_STRING    "3.0"
> #define SMB302_VERSION_STRING    "3.02"
> #define SMB31_VERSION_STRING    "3.1"
>
>
> The matching works as expected, e,g. specifying 3.0 gets matched to
> Smb_30, before and/after adding the sixth element to the match_table_t
> - except that unmatched items (picking a dialect that doesn't exist
> like "5.1") matches to Smb_21 where it used to fall through to the
> default (error) case.
>
> It got me a little worried that there MAX_OPT_ARGS is 3 and I am
> getting the third element in the case of the error.
>
> Looking at other examples in the kernel were strange e.g. ext4/super.c has this
>
>         /*
>          * Initialize args struct so we know whether arg was
>          * found; some options take optional arguments.
>          */
>         args[0].to = args[0].from = NULL;
>         token = match_token(p, tokens, args);
>         (and then passes args down to a large helper function handle_mount_opt)
>
> Initializing args didn't seem to help in the cifs case
>
> On Wed, Sep 17, 2014 at 3:36 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> 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
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: match_token weird behavior
       [not found]               ` <CAH2r5muvhC6ww_U8o4mN119ptnNsuT+mcSmfaFbP7DqV0ZQpiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-09-18 17:10                 ` Steve French
  0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2014-09-18 17:10 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: linux-fsdevel, LKML,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Viro

On Wed, Sep 17, 2014 at 2:05 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Looking at more examples, some of which are much larger match tables
> maybe it has to do with how the final entry is defined.  In this
> example the NULL match is explicitly stated.
>
> static const match_table_t cifs_secflavor_tokens = {
>     { Opt_sec_krb5, "krb5" },
>     { Opt_sec_krb5i, "krb5i" },
>     { Opt_sec_krb5p, "krb5p" },
>     { Opt_sec_ntlmsspi, "ntlmsspi" },
>     { Opt_sec_ntlmssp, "ntlmssp" },
>     { Opt_ntlm, "ntlm" },
>     { Opt_sec_ntlmi, "ntlmi" },
>     { Opt_sec_ntlmv2, "nontlm" },
>     { Opt_sec_ntlmv2, "ntlmv2" },
>     { Opt_sec_ntlmv2i, "ntlmv2i" },
>     { Opt_sec_lanman, "lanman" },
>     { Opt_sec_none, "none" },
>
>     { Opt_sec_err, NULL }
> };


Adding in a dummy entry at the end of the match_table_t did fix the problem.

{ Smb_version_err, NULL }

Problem solved.

> On Wed, Sep 17, 2014 at 3:53 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> For additional information the strings that are being matched against are:
>>
>> #define SMB1_VERSION_STRING    "1.0"
>> #define SMB20_VERSION_STRING    "2.0"
>> #define SMB21_VERSION_STRING    "2.1"
>> #define SMB30_VERSION_STRING    "3.0"
>> #define SMB302_VERSION_STRING    "3.02"
>> #define SMB31_VERSION_STRING    "3.1"
>>
>>
>> The matching works as expected, e,g. specifying 3.0 gets matched to
>> Smb_30, before and/after adding the sixth element to the match_table_t
>> - except that unmatched items (picking a dialect that doesn't exist
>> like "5.1") matches to Smb_21 where it used to fall through to the
>> default (error) case.
>>
>> It got me a little worried that there MAX_OPT_ARGS is 3 and I am
>> getting the third element in the case of the error.
>>
>> Looking at other examples in the kernel were strange e.g. ext4/super.c has this
>>
>>         /*
>>          * Initialize args struct so we know whether arg was
>>          * found; some options take optional arguments.
>>          */
>>         args[0].to = args[0].from = NULL;
>>         token = match_token(p, tokens, args);
>>         (and then passes args down to a large helper function handle_mount_opt)
>>
>> Initializing args didn't seem to help in the cifs case
>>
>> On Wed, Sep 17, 2014 at 3:36 PM, Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>>> 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
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-18 17:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-17 18:20 match_token weird behavior Steve French
     [not found] ` <CAH2r5mvTP1oGNoetuSkPdirBb1UDH6fiNCVjs_gJNKeJ1Dz5sA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-17 20:33   ` Randy Dunlap
     [not found]     ` <5419F007.9070509-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-09-17 20:36       ` Randy Dunlap
     [not found]         ` <5419F0D4.9090107-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-09-17 20:53           ` Steve French
2014-09-17 21:05             ` Steve French
     [not found]               ` <CAH2r5muvhC6ww_U8o4mN119ptnNsuT+mcSmfaFbP7DqV0ZQpiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-18 17:10                 ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).