On Tue, 2017-08-08 at 12:06 -0400, Alan Stern wrote:
> On Tue, 8 Aug 2017, Hannes Reinecke wrote:
> > On 08/08/2017 05:25 PM, Alan Stern wrote:
> > > On Tue, 8 Aug 2017, Hannes Reinecke wrote:
> > >
> > > > When checking the model and vendor string we need to use the
> > > > minimum value of either string, otherwise we'll miss out on
> > > > wildcard matches.
> > > > And we should avoid matching anything with zero size as this
> > > > match will be incorrect.
> > > > Without this patch certain Hitachi arrays will not be presenting
> > > > VPD pages correctly.
> > > >
> > > > Fixes: 5e7ff2c ("SCSI: fix new bug in scsi_dev_info_list string
> > > > matching")
> > > > Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> > >
> > > This is very questionable. As described in commit b704f70ce200
> > > ("SCSI:
> > > fix bug in scsi_dev_info_list matching"), there are devices which
> > > have
> > > vendor or model strings of length 0, and there are entries in the
> > > global device list like that too. With this patch, such devices and
> > > such entries would never match anything.
> > >
> > > Also, the idea of using the minimum length of either string doesn't
> > > sound right. It would allow a device with string "abc" to match an
> > > entry in the list with string "abcd", which would be wrong.
> > >
> > > Suggestion:
> > >
> > > Allow a device string to match a list entry if the lengths
> > > are equal or the list entry is shorter (this would be a
> > > wildcard match), but not if the list entry is longer. Here
> > > the device string length is after leading and trailing blanks
> > > have been removed.
> > >
> > > As a corollary, allow an empty device string to match an empty
> > > list entry but nothing else.
> > >
> > > Alan Stern
> > >
> >
> > Well, so far we haven't encountered any of those devices.
>
> But what if we do in the future? The code should be correct for
> anything.
>
> > I _have_ validated the patch with the known problematic inquiry strings,
> > namely
> >
> > "HITACHI" "OPEN-V"
> > "Inateck" " "
> > " " "Scanner"
> > "Promise" "STEX"
> >
> > and it gives the correct result for all of these.
>
> How can " " "Scanner" give the correct result? Oh, I see -- the
> patch description and the code comment both say that empty strings
> never match, but in fact they do -- you allow a match if either the
> vendor or model string is empty, but not if they are both empty.
>
> > If you have any inquiry strings for which this does _not_ produce the
> > correct result please let me know.
>
> What about "HITA" "OPEN-V"? Or "" "OPEN-V"? Both of these will match
> the "HITACHI" "OPEN-" entry, but neither of them should.
>
> What about "ABCD" "Scanner"? This will match the " " "Scanner"
> entry, but it shouldn't.
Hello Hannes,
Is there any case for which you think it is useful to perform a prefix match
on the vendor name? What would break if the code for comparing the vendor
name would be changed into an exact match (after having trimmed trailing
whitespace)?
Thanks,
Bart.
