Re: [scsi:for-next 200/200] drivers/scsi/scsi_sysfs.c:778: warning: comparison is always false due to limited range of data type

Re: [scsi:for-next 200/200] drivers/scsi/scsi_sysfs.c:778: warning: comparison is always false due to limited range of data type

[James Bottomley]

[correct cc to linux-scsi added]
On Thu, 2014-03-20 at 12:48 +0800, kbuild test robot wrote:
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
> head:   c6b7d7a684ef39be73f06c14aca4aadad77f42dc
> commit: c6b7d7a684ef39be73f06c14aca4aadad77f42dc [200/200] [SCSI] Invalidate VPD page data
> config: make ARCH=avr32 atstk1002_defconfig
> 
> All warnings:
> 
>    drivers/scsi/scsi_sysfs.c: In function 'show_vpd_pg83':
> >> drivers/scsi/scsi_sysfs.c:778: warning: comparison is always false due to limited range of data type
>    drivers/scsi/scsi_sysfs.c: In function 'show_vpd_pg80':
> >> drivers/scsi/scsi_sysfs.c:779: warning: comparison is always false due to limited range of data type
> 
> vim +778 drivers/scsi/scsi_sysfs.c
> 
> beba1fc0 Hannes Reinecke 2014-03-15  772  static struct bin_attribute dev_attr_vpd_##_page = {		\
> beba1fc0 Hannes Reinecke 2014-03-15  773  	.attr =	{.name = __stringify(vpd_##_page), .mode = S_IRUGO },	\
> beba1fc0 Hannes Reinecke 2014-03-15  774  	.size = 0,							\
> beba1fc0 Hannes Reinecke 2014-03-15  775  	.read = show_vpd_##_page,					\
> beba1fc0 Hannes Reinecke 2014-03-15  776  };
> beba1fc0 Hannes Reinecke 2014-03-15  777  
> beba1fc0 Hannes Reinecke 2014-03-15 @778  sdev_vpd_pg_attr(pg83);
> b6159c63 Hannes Reinecke 2014-03-15 @779  sdev_vpd_pg_attr(pg80);
> beba1fc0 Hannes Reinecke 2014-03-15  780  
> ^1da177e Linus Torvalds  2005-04-16  781  static ssize_t
> beba1fc0 Hannes Reinecke 2014-03-15  782  show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
> 
> :::::: The code at line 778 was first introduced by commit
> :::::: beba1fc005a56a0597d01d3f381e533427856f5d [SCSI] Add EVPD page 0x83 to sysfs
> 
> :::::: TO: Hannes Reinecke <hare@suse.de>
> :::::: CC: James Bottomley <JBottomley@Parallels.com>

Actually, it's not just this that's a problem.  Since the _len variables
are unsigned chars, they're bogus in more ways than having errors put
into them because the lengths of the pages can go over 255 as well.

Bumping them up to int should fix this.  However, the way we go about
getting the vpd pages looks wrong.  You should reset vpd_len back to the
intial value (255) after obtaining each page, shouldn't you, in order to
avoid potentially wasting buffer space (unless we're assuming the pages
always get larger)?

James

Re: [scsi:for-next 200/200] drivers/scsi/scsi_sysfs.c:778: warning: comparison is always false due to limited range of data type

[Hannes Reinecke]

On 03/20/2014 06:07 AM, James Bottomley wrote:
> [correct cc to linux-scsi added]
> On Thu, 2014-03-20 at 12:48 +0800, kbuild test robot wrote:
>> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
>> head:   c6b7d7a684ef39be73f06c14aca4aadad77f42dc
>> commit: c6b7d7a684ef39be73f06c14aca4aadad77f42dc [200/200] [SCSI] Invalidate VPD page data
>> config: make ARCH=avr32 atstk1002_defconfig
>>
>> All warnings:
>>
>>    drivers/scsi/scsi_sysfs.c: In function 'show_vpd_pg83':
>>>> drivers/scsi/scsi_sysfs.c:778: warning: comparison is always false due to limited range of data type
>>    drivers/scsi/scsi_sysfs.c: In function 'show_vpd_pg80':
>>>> drivers/scsi/scsi_sysfs.c:779: warning: comparison is always false due to limited range of data type
>>
>> vim +778 drivers/scsi/scsi_sysfs.c
>>
>> beba1fc0 Hannes Reinecke 2014-03-15  772  static struct bin_attribute dev_attr_vpd_##_page = {		\
>> beba1fc0 Hannes Reinecke 2014-03-15  773  	.attr =	{.name = __stringify(vpd_##_page), .mode = S_IRUGO },	\
>> beba1fc0 Hannes Reinecke 2014-03-15  774  	.size = 0,							\
>> beba1fc0 Hannes Reinecke 2014-03-15  775  	.read = show_vpd_##_page,					\
>> beba1fc0 Hannes Reinecke 2014-03-15  776  };
>> beba1fc0 Hannes Reinecke 2014-03-15  777  
>> beba1fc0 Hannes Reinecke 2014-03-15 @778  sdev_vpd_pg_attr(pg83);
>> b6159c63 Hannes Reinecke 2014-03-15 @779  sdev_vpd_pg_attr(pg80);
>> beba1fc0 Hannes Reinecke 2014-03-15  780  
>> ^1da177e Linus Torvalds  2005-04-16  781  static ssize_t
>> beba1fc0 Hannes Reinecke 2014-03-15  782  show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
>>
>> :::::: The code at line 778 was first introduced by commit
>> :::::: beba1fc005a56a0597d01d3f381e533427856f5d [SCSI] Add EVPD page 0x83 to sysfs
>>
>> :::::: TO: Hannes Reinecke <hare@suse.de>
>> :::::: CC: James Bottomley <JBottomley@Parallels.com>
> 
> Actually, it's not just this that's a problem.  Since the _len variables
> are unsigned chars, they're bogus in more ways than having errors put
> into them because the lengths of the pages can go over 255 as well.
> 
> Bumping them up to int should fix this.  However, the way we go about
> getting the vpd pages looks wrong.  You should reset vpd_len back to the
> intial value (255) after obtaining each page, shouldn't you, in order to
> avoid potentially wasting buffer space (unless we're assuming the pages
> always get larger)?
> 
Hmm. In principle, yes. However in practice VPD page 0x83 tends to
be the largest of them, so we'd only modifying the page length for
that. And then it doesn't matter.

But yeah, I'll be fixing it up.

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