On Fri, 31 Jul 2015, Giulio Bernardi wrote:
> On 31/07/2015 18:05, Alan Stern wrote:
> > On Thu, 30 Jul 2015, Giulio Bernardi wrote:
> >
> >>> You should check that scsi_report_lun_scan() really does return 1. If
> >>> it returns 0 instead (because BLIST_NOLUN is set in bflags or for any
> >>> other reason), that would cause the behavior you see.
> >>>
> >>
> >> I found the reason of this behavior: there is BLIST_NOLUN in bflags indeed.
> >> It is a bug in scsi_get_device_flags_keyed():
> >> In fact, my device reports these strings for model and vendor:
> >> vendor = "Inateck 0101"
> >> model = " 0101"
> >
> > That's not quite right. The vendor string is always exactly 8 bytes,
> > the model string is always exactly 16 bytes, and the revision string is
> > always exactly 4 bytes. Therefore your device actually reports vendor
> > = "Inateck ", model = " ", and revision = "0101".
> >
> >> and this matches with this entry in the global scsi device list:
> >> {"", "Scanner", "1.80", BLIST_NOLUN}
> >> In fact: the check on vendor passes because strlen(devinfo->vendor) == 0 so
> >> memcmp always passes, and the check on model passes because the code assumes
> >> that max length of model is 16: when trimming the string (which is exactly 16
> >> spaces followed by 0101) it ends up with an empty string, which then matches
> >> whatever string for the same reason of the previous check.
> >
> > Yes, both of those checks are buggy. I'm surprised they weren't fixed
> > long ago.
> >
> >> I am recompiling the kernel now, with max = 20 instead of max = 16 to see if it
> >> works, but I don't know if other devices do exist with a longer model string.
> >> Maybe those limits (8 for vendor, 16 for model) were true for real scsi and not
> >> for USB devices?
> >
> > Those limits always hold for all SCSI devices, including USB. They are
> > part of the SCSI specification. The right way to fix this is to change
> > the tests, not to increase the max string length. Does the patch below
> > fix the problem for you?
> >
>
> I tried the patch but it doesn't work. I applied it without looking at it
> before, but the problem is that actually "vendor" variable is really "Inateck
> 0101" (28 chars) and "model" is really " 0101" (20
> chars): at least, that's the data that is passed to scsi_get_device_flags_keyed().
Those variables are _not_ NUL-terminated strings. They are simply
character arrays. model is an array of 8 characters and vendor is an
array of 16 characters.
> So your code is never executed
> + while (max > 0 && model[max - 1] == ' ')
> + --max;
> + if (memcmp(devinfo->model, model, max) ||
> + devinfo->model[max])
> because max is already 0 from the previous loop.
You mean that the "while" loop terminates immediately. That's okay;
it's supposed to do that when every character in model is ' '.
The memcmp call should return 0 because max is 0. But since
devinfo->model[0] is 'S' (the first letter of "Scanner"), the "if"
statement should succeed. Therefore your device should not match the
devinfo entry.
> Maybe I should see where vendor and model come from? Maybe they are concatenated
> from the original vendor and model values reported from the device, so that
> might be why those strings are so long.
That is exactly where they come from. The device does not report
strings, however. It reports an array of 28 characters (8 for the
vendor, 16 for the model, and 4 for the revision).
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
