Re: [PATCH 2/5] scsi: Return VPD page length in scsi_vpd_inquiry()

[Hannes Reinecke <hare@suse.de>]

On 03/12/2014 10:14 PM, Muthukumar R wrote:
> On Mon, Mar 10, 2014 at 7:28 AM, Hannes Reinecke <hare@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 <hare@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
> 
> 
Ah. Correct.

>>  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;
> 
> 
No. See above. The actual data starts at offset 4.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html