Re: [PATCHv3 5/5] scsi_devinfo: fixup string compare

Hannes Reinecke
2017-08-16T05:57:38Z

On 08/16/2017 01:03 AM, Bart Van Assche wrote:
> On Fri, 2017-08-11 at 16:23 +0200, 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 take card when matching with zero size strings;
>
> "card"? Did you perhaps mean "care"?
>
Yes, indeed.

>> results might be unpredictable.
>> With this patch the rules for matching devinfo strings are
>> as follows:
>> - Vendor strings must match exactly
>> - Empty Model strings will only match if the devinfo model
>> is also empty
>> - Model strings shorter than the devinfo model string will
>> not match
>
> The above sentence contradicts the first sentence.
>
Ah. Will be rewriting this.

>> + /*
>> + * @model specifies the full string, and
>> + * must be larger or equal to devinfo->model
>> + */
>
> What does "larger than or equal to" mean for strings? Did you perhaps mean that
> devinfo->model must be a prefix of @model?
>
Yes.

>> if (!memcmp(devinfo->vendor, vendor,
>> - sizeof(devinfo->vendor)) &&
>> - !memcmp(devinfo->model, model,
>> - sizeof(devinfo->model)))
>> + sizeof(devinfo->vendor)) &&
>> + !memcmp(devinfo->model, model,
>> + sizeof(devinfo->model)))
>> return devinfo;
>
> Is this a whitespace-only change? If so, please fold it in the previous patch.
>
Ok.

Cheers,

Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)