On Wed, Mar 12, 2014 at 2:43 PM, Douglas Gilbert <dgilb...@interlog.com> wrote:
> On 14-03-12 10:14 PM, Muthukumar R wrote:
>>
>> On Mon, Mar 10, 2014 at 7:28 AM, Hannes Reinecke <h...@suse.de> wrote:
>>>
>>> We should be returning the number of bytes of the
>>> requested VPD page in scsi_vpd_inquiry.
>>> This makes it easier for the caller to verify the
>>> required space.
>>>
>>> Signed-off-by: Hannes Reinecke <h...@suse.de>
>>> ---
>>>   drivers/scsi/scsi.c | 17 ++++++++++-------
>>>   1 file changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>>> index d8afec8..ecaeff1 100644
>>> --- a/drivers/scsi/scsi.c
>>> +++ b/drivers/scsi/scsi.c
>>> @@ -954,7 +954,7 @@ EXPORT_SYMBOL(scsi_track_queue_full);
>>>    * This is an internal helper function.  You probably want to use
>>>    * scsi_get_vpd_page instead.
>>>    *
>>> - * Returns 0 on success or a negative error number.
>>> + * Returns size of the vpg page on success or a negative error number.
>>>    */
>>
>> Typo:                              ^^^
>>
>>   Should be: vpd page
>>
>>
>>>   static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char
>>> *buffer,
>>>                                                          u8 page,
>>> unsigned len)
>>> @@ -962,6 +962,9 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev,
>>> unsigned char *buffer,
>>>          int result;
>>>          unsigned char cmd[16];
>>>
>>> +       if (len < 4)
>>> +               return -EINVAL;
>>> +
>>>          cmd[0] = INQUIRY;
>>>          cmd[1] = 1;             /* EVPD */
>>>          cmd[2] = page;
>>> @@ -982,7 +985,7 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev,
>>> unsigned char *buffer,
>>>          if (buffer[1] != page)
>>>                  return -EIO;
>>>
>>> -       return 0;
>>> +       return get_unaligned_be16(&buffer[2]) + 4;
>>>   }
>>>
>>>   /**
>>> @@ -1009,18 +1012,18 @@ int scsi_get_vpd_page(struct scsi_device *sdev,
>>> u8 page, unsigned char *buf,
>>>
>>>          /* Ask for all the pages supported by this device */
>>>          result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
>>> -       if (result)
>>> +       if (result < 4)
>>>                  goto fail;
>>
>>
>>
>> You mean:
>>
>>              if (result < 0)
>>                     goto fail;
>
>
> I think that he means:
>
>      if (result < 4)
>           goto fail;
>
> The first 4 bytes of a VPD page are a header with buf[2] and
> buf[3] being the length of the following payload. Without
> both of them "fail" is a quite accurate description.
>
> That said, it is hard to get a number between 0 and 3 with:
>
>    return get_unaligned_be16(&buffer[2]) + 4;
> but if you wanted to take resid into account it is possible.
>
> IMO this snippet is fine.
>

I see.. thanks. Comment is slightly misleading but I guess its OK in this case.


>
> Doug Gilbert
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to