Re: [PATCH 2/3] data_smart: Allow numeric characters in overrides

[richard.purdie@linuxfoundation.org]

On Tue, 2018-12-18 at 21:05 +0000, Peter Kjellerstedt wrote:
> I do not understand this commit. The commit explanation and the code change 
> don't match up. The motivation in the commit message is that there was 
> problems with overrides such as x86-64 containing digits and that the 
> code should be changed to allow overrides to be lower case and numeric.
> However, the modified code changes the test from using islower(), which as 
> far as I can tell only validates the alpha-characters in the string and 
> ignores all else, to using a regular expression r'[a-z0-9]+', which will 
> match as long as the first character is a lower case character or a numeric 
> character (note that there is no $ at the end of the regular expression).
> 
> Here are tests with islower(), which correctly validates "x86" and
> "x86-64" 
> as lower case overrides, but not "fooBar":
> 
> > > > "x86".islower()
> True
> > > > "x86-64".islower()
> True
> > > > "fooBar".islower()
> False
> 
> Here are the corresponding tests using the r'[a-z0-9]+' regular
> expression.
> Note that it only matches "x86" from "x86-64" and "foo" from
> "fooBar", and 
> will incorrectly allow "fooBar" as an override:
> 
> > > > re.match(r'[a-z0-9]+', "x86")
> <_sre.SRE_Match object; span=(0, 3), match='x86'>
> > > > re.match(r'[a-z0-9]+', "x86-64")
> <_sre.SRE_Match object; span=(0, 3), match='x86'>
> > > > re.match(r'[a-z0-9]+', "fooBar")
> <_sre.SRE_Match object; span=(0, 3), match='foo'>
> 
> So why change from islower(), which as far as I can tell did the
> right thing from the beginning?

The commit is clearly wrong. The intent of the commit message is
correct, it doesn't match what that regex actually does :(

Cheers,

Richard