From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756914AbaIQUgm (ORCPT ); Wed, 17 Sep 2014 16:36:42 -0400 Received: from casper.infradead.org ([85.118.1.10]:60352 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756245AbaIQUgk (ORCPT ); Wed, 17 Sep 2014 16:36:40 -0400 Message-ID: <5419F0D4.9090107@infradead.org> Date: Wed, 17 Sep 2014 13:36:36 -0700 From: Randy Dunlap User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: Steve French , linux-fsdevel , LKML CC: "linux-cifs@vger.kernel.org" , Al Viro Subject: Re: match_token weird behavior References: <5419F007.9070509@infradead.org> In-Reply-To: <5419F007.9070509@infradead.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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