linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning
@ 2016-06-15 20:42 Arnd Bergmann
  2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-06-15 20:42 UTC (permalink / raw)
  To: James E.J. Bottomley, Martin K. Petersen
  Cc: Arnd Bergmann, James Smart, Dick Kennedy, Hannes Reinicke,
	James Bottomley, linux-scsi, linux-kernel

When building with -Wextra, we get a lot of warnings for the lpfc driver
concerning expressions that are always true, starting with:

drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_npiv_init':
drivers/scsi/lpfc/lpfc_attr.c:2786:77: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_rrq_init':
drivers/scsi/lpfc/lpfc_attr.c:2802:76: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_suppress_link_up_init':
drivers/scsi/lpfc/lpfc_attr.c:2812:2050: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_log_verbose_init':
drivers/scsi/lpfc/lpfc_attr.c:3064:1930: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]

The code works as intented, but it would be nice to shut up the
warning so we don't clutter up build logs with this. Using a
separate inline function for it makes it clear to the compiler
that the comparison is necessary in the caller but still lets
it do the constant-folding.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/lpfc/lpfc_attr.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index cfec2eca4dd3..3e1d2e669902 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -1620,6 +1620,11 @@ lpfc_sriov_hw_max_virtfn_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", max_nr_virtfn);
 }
 
+static inline bool lpfc_rangecheck(uint val, uint min, uint max)
+{
+	return val >= min && val <= max;
+}
+
 /**
  * lpfc_param_show - Return a cfg attribute value in decimal
  *
@@ -1697,7 +1702,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \
 static int \
 lpfc_##attr##_init(struct lpfc_hba *phba, uint val) \
 { \
-	if (val >= minval && val <= maxval) {\
+	if (lpfc_rangecheck(val, minval, maxval)) {\
 		phba->cfg_##attr = val;\
 		return 0;\
 	}\
@@ -1732,7 +1737,7 @@ lpfc_##attr##_init(struct lpfc_hba *phba, uint val) \
 static int \
 lpfc_##attr##_set(struct lpfc_hba *phba, uint val) \
 { \
-	if (val >= minval && val <= maxval) {\
+	if (lpfc_rangecheck(val, minval, maxval)) {\
 		lpfc_printf_log(phba, KERN_ERR, LOG_INIT, \
 			"3052 lpfc_" #attr " changed from %d to %d\n", \
 			phba->cfg_##attr, val); \
@@ -1856,7 +1861,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \
 static int \
 lpfc_##attr##_init(struct lpfc_vport *vport, uint val) \
 { \
-	if (val >= minval && val <= maxval) {\
+	if (lpfc_rangecheck(val, minval, maxval)) {\
 		vport->cfg_##attr = val;\
 		return 0;\
 	}\
@@ -1888,7 +1893,7 @@ lpfc_##attr##_init(struct lpfc_vport *vport, uint val) \
 static int \
 lpfc_##attr##_set(struct lpfc_vport *vport, uint val) \
 { \
-	if (val >= minval && val <= maxval) {\
+	if (lpfc_rangecheck(val, minval, maxval)) {\
 		lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT, \
 			"3053 lpfc_" #attr \
 			" changed from %d (x%x) to %d (x%x)\n", \
-- 
2.9.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] scsi: wd7000: print sector number as 64-bit
  2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
@ 2016-06-15 20:54 ` Arnd Bergmann
  2016-06-16 17:36   ` kbuild test robot
  2016-06-21  1:12   ` Martin K. Petersen
  2016-06-16  7:39 ` [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Johannes Thumshirn
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-06-15 20:54 UTC (permalink / raw)
  To: James E.J. Bottomley, linux-scsi; +Cc: Martin K. Petersen, linux-kernel

Enabling format checking in dprintk() shows that wd7000_biosparam
uses an incorrect format string for sector_t:

drivers/scsi/wd7000.c: In function 'wd7000_biosparam':
drivers/scsi/wd7000.c:1594:21: error: format '%d' expects argument of type 'int', but argument 3 has type 'sector_t {aka long long unsigned int}' [-Werror=format=]

As sector_t can be 32-bit wide, this adds a cast to 'u64' and prints
that with the correct format. The change to use no_printk()
generally helps with finding this kind of hidden format string bug,
and I found that when building with "-Wextra", which warned about
an empty else clause in

       } else
              dprintk("ok!\n");

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Not Cc'ing Miroslav Zagorac <zaga@fly.cc.fer.hr>, that address bounces
with "Requested action not taken: mailbox unavailable invalid DNS MX or A/AAAA resource record"

diff --git a/drivers/scsi/wd7000.c b/drivers/scsi/wd7000.c
index 0c0f17b9a3eb..39a3b5333bd6 100644
--- a/drivers/scsi/wd7000.c
+++ b/drivers/scsi/wd7000.c
@@ -192,7 +192,7 @@
 #ifdef WD7000_DEBUG
 #define dprintk printk
 #else
-#define dprintk(format,args...)
+#define dprintk	no_printk
 #endif
 
 /*
@@ -1591,8 +1591,8 @@ static int wd7000_biosparam(struct scsi_device *sdev,
 {
 	char b[BDEVNAME_SIZE];
 
-	dprintk("wd7000_biosparam: dev=%s, size=%d, ",
-		bdevname(bdev, b), capacity);
+	dprintk("wd7000_biosparam: dev=%s, size=%lld, ",
+		bdevname(bdev, b), (s64)capacity);
 	(void)b;	/* unused var warning? */
 
 	/*

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning
  2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
  2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
@ 2016-06-16  7:39 ` Johannes Thumshirn
  2016-07-14  3:15 ` Martin K. Petersen
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2016-06-16  7:39 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Hannes Reinicke, James Bottomley, linux-scsi,
	linux-kernel

On Wed, Jun 15, 2016 at 10:42:17PM +0200, Arnd Bergmann wrote:
> When building with -Wextra, we get a lot of warnings for the lpfc driver
> concerning expressions that are always true, starting with:
> 
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_npiv_init':
> drivers/scsi/lpfc/lpfc_attr.c:2786:77: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_rrq_init':
> drivers/scsi/lpfc/lpfc_attr.c:2802:76: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_suppress_link_up_init':
> drivers/scsi/lpfc/lpfc_attr.c:2812:2050: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_log_verbose_init':
> drivers/scsi/lpfc/lpfc_attr.c:3064:1930: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> 
> The code works as intented, but it would be nice to shut up the
> warning so we don't clutter up build logs with this. Using a
> separate inline function for it makes it clear to the compiler
> that the comparison is necessary in the caller but still lets
> it do the constant-folding.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] scsi: wd7000: print sector number as 64-bit
  2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
@ 2016-06-16 17:36   ` kbuild test robot
  2016-06-17 11:45     ` Arnd Bergmann
  2016-06-21  1:12   ` Martin K. Petersen
  1 sibling, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2016-06-16 17:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kbuild-all, James E.J. Bottomley, linux-scsi, Martin K. Petersen,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 6385 bytes --]

Hi,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.7-rc3 next-20160616]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/scsi-lpfc-avoid-harmless-comparison-warning/20160616-045453
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   drivers/scsi/wd7000.c: In function 'mail_out':
   drivers/scsi/wd7000.c:915:44: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
       any2scsi((unchar *) ogmbs[ogmb].scbptr, (int) scbptr);
                                               ^
   drivers/scsi/wd7000.c: In function 'wd7000_queuecommand_lck':
   drivers/scsi/wd7000.c:1117:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      any2scsi(scb->dataptr, (int) sgb);
                             ^
   drivers/scsi/wd7000.c:1121:4: warning: 'isa_page_to_bus' is deprecated [-Wdeprecated-declarations]
       any2scsi(sgb[i].ptr, isa_page_to_bus(sg_page(sg)) + sg->offset);
       ^
   In file included from include/linux/io.h:25:0,
                    from include/linux/irq.h:24,
                    from include/asm-generic/hardirq.h:12,
                    from arch/alpha/include/asm/hardirq.h:7,
                    from include/linux/hardirq.h:8,
                    from include/linux/interrupt.h:12,
                    from drivers/scsi/wd7000.c:170:
   arch/alpha/include/asm/io.h:95:39: note: declared here
    static inline dma_addr_t __deprecated isa_page_to_bus(struct page *page)
                                          ^
   drivers/scsi/wd7000.c:1128:4: warning: 'isa_page_to_bus' is deprecated [-Wdeprecated-declarations]
       any2scsi(scb->dataptr, isa_page_to_bus(sg_page(sg)) + sg->offset);
       ^
   In file included from include/linux/io.h:25:0,
                    from include/linux/irq.h:24,
                    from include/asm-generic/hardirq.h:12,
                    from arch/alpha/include/asm/hardirq.h:7,
                    from include/linux/hardirq.h:8,
                    from include/linux/interrupt.h:12,
                    from drivers/scsi/wd7000.c:170:
   arch/alpha/include/asm/io.h:95:39: note: declared here
    static inline dma_addr_t __deprecated isa_page_to_bus(struct page *page)
                                          ^
   drivers/scsi/wd7000.c: In function 'wd7000_diagnostics':
   drivers/scsi/wd7000.c:1151:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     any2scsi(icb.ptr, (int) &buf);
                       ^
   drivers/scsi/wd7000.c: In function 'wd7000_adapter_reset':
   drivers/scsi/wd7000.c:1236:46: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     any2scsi((unchar *) & (init_cmd.mailboxes), (int) &(host->mb));
                                                 ^
   In file included from include/linux/kernel.h:13:0,
                    from include/linux/delay.h:10,
                    from drivers/scsi/wd7000.c:168:
   drivers/scsi/wd7000.c: In function 'wd7000_detect':
   drivers/scsi/wd7000.c:1482:59: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
        dprintk("wd7000_detect: adapter allocated at 0x%x\n", (int) host);
                                                              ^
   include/linux/printk.h:114:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__); \
                    ^
>> drivers/scsi/wd7000.c:1482:5: note: in expansion of macro 'dprintk'
        dprintk("wd7000_detect: adapter allocated at 0x%x\n", (int) host);
        ^

vim +/dprintk +1482 drivers/scsi/wd7000.c

^1da177e Linus Torvalds 2005-04-16  1466  				dprintk("ok!\n");
^1da177e Linus Torvalds 2005-04-16  1467  
^1da177e Linus Torvalds 2005-04-16  1468  			if (inb(iobase + ASC_INTR_STAT) == 1) {
^1da177e Linus Torvalds 2005-04-16  1469  				/*
^1da177e Linus Torvalds 2005-04-16  1470  				 *  We register here, to get a pointer to the extra space,
^1da177e Linus Torvalds 2005-04-16  1471  				 *  which we'll use as the Adapter structure (host) for
^1da177e Linus Torvalds 2005-04-16  1472  				 *  this adapter.  It is located just after the registered
^1da177e Linus Torvalds 2005-04-16  1473  				 *  Scsi_Host structure (sh), and is located by the empty
^1da177e Linus Torvalds 2005-04-16  1474  				 *  array hostdata.
^1da177e Linus Torvalds 2005-04-16  1475  				 */
^1da177e Linus Torvalds 2005-04-16  1476  				sh = scsi_register(tpnt, sizeof(Adapter));
^1da177e Linus Torvalds 2005-04-16  1477  				if (sh == NULL)
^1da177e Linus Torvalds 2005-04-16  1478  					goto err_release;
^1da177e Linus Torvalds 2005-04-16  1479  
^1da177e Linus Torvalds 2005-04-16  1480  				host = (Adapter *) sh->hostdata;
^1da177e Linus Torvalds 2005-04-16  1481  
^1da177e Linus Torvalds 2005-04-16 @1482  				dprintk("wd7000_detect: adapter allocated at 0x%x\n", (int) host);
^1da177e Linus Torvalds 2005-04-16  1483  				memset(host, 0, sizeof(Adapter));
^1da177e Linus Torvalds 2005-04-16  1484  
^1da177e Linus Torvalds 2005-04-16  1485  				host->irq = configs[pass].irq;
^1da177e Linus Torvalds 2005-04-16  1486  				host->dma = configs[pass].dma;
^1da177e Linus Torvalds 2005-04-16  1487  				host->iobase = iobase;
^1da177e Linus Torvalds 2005-04-16  1488  				host->int_counter = 0;
^1da177e Linus Torvalds 2005-04-16  1489  				host->bus_on = configs[pass].bus_on;
^1da177e Linus Torvalds 2005-04-16  1490  				host->bus_off = configs[pass].bus_off;

:::::: The code at line 1482 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46344 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] scsi: wd7000: print sector number as 64-bit
  2016-06-16 17:36   ` kbuild test robot
@ 2016-06-17 11:45     ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-06-17 11:45 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, James E.J. Bottomley, linux-scsi, Martin K. Petersen,
	linux-kernel

On Friday, June 17, 2016 1:36:52 AM CEST kbuild test robot wrote:
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/scsi/wd7000.c: In function 'mail_out':
>    drivers/scsi/wd7000.c:915:44: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>        any2scsi((unchar *) ogmbs[ogmb].scbptr, (int) scbptr);
>                                                ^
>    drivers/scsi/wd7000.c: In function 'wd7000_queuecommand_lck':
>    drivers/scsi/wd7000.c:1117:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>       any2scsi(scb->dataptr, (int) sgb);
>                              ^
>    drivers/scsi/wd7000.c:1121:4: warning: 'isa_page_to_bus' is deprecated [-Wdeprecated-declarations]
>    drivers/scsi/wd7000.c:1128:4: warning: 'isa_page_to_bus' is deprecated [-Wdeprecated-declarations]
>    drivers/scsi/wd7000.c: In function 'wd7000_diagnostics':
>    drivers/scsi/wd7000.c:1151:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>    drivers/scsi/wd7000.c:1236:46: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>    drivers/scsi/wd7000.c:1482:59: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

I'm pretty sure these are all preexisting warnings, and they don't show up
on ARM or x86.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] scsi: wd7000: print sector number as 64-bit
  2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
  2016-06-16 17:36   ` kbuild test robot
@ 2016-06-21  1:12   ` Martin K. Petersen
  2016-06-21 11:41     ` Arnd Bergmann
  1 sibling, 1 reply; 10+ messages in thread
From: Martin K. Petersen @ 2016-06-21  1:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: James E.J. Bottomley, linux-scsi, Martin K. Petersen, linux-kernel

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

As sector_t can be 32-bit wide, this adds a cast to 'u64' and prints
that with the correct format. The change to use no_printk()

[...]

+	dprintk("wd7000_biosparam: dev=%s, size=%lld, ",
+		bdevname(bdev, b), (s64)capacity);

s64?

Why not unsigned long long like we usually do with sector_t?

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] scsi: wd7000: print sector number as 64-bit
  2016-06-21  1:12   ` Martin K. Petersen
@ 2016-06-21 11:41     ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-06-21 11:41 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: James E.J. Bottomley, linux-scsi, linux-kernel

On Monday, June 20, 2016 9:12:50 PM CEST Martin K. Petersen wrote:
> >>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:
> 
> As sector_t can be 32-bit wide, this adds a cast to 'u64' and prints
> that with the correct format. The change to use no_printk()
> 
> [...]
> 
> +       dprintk("wd7000_biosparam: dev=%s, size=%lld, ",
> +               bdevname(bdev, b), (s64)capacity);
> 
> s64?
> 
> Why not unsigned long long like we usually do with sector_t?

The printk() was using %d as the format string, printing it
as a signed number, so I did not change it.

Using %llu makes sense though, I've resent it like that.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning
  2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
  2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
  2016-06-16  7:39 ` [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Johannes Thumshirn
@ 2016-07-14  3:15 ` Martin K. Petersen
  2016-07-15 19:16 ` James Smart
  2016-07-20 23:54 ` Martin K. Petersen
  4 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2016-07-14  3:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Hannes Reinicke, linux-scsi, linux-kernel

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> When building with -Wextra, we get a lot of warnings for the lpfc
Arnd> driver concerning expressions that are always true, starting with:

Arnd> drivers/scsi/lpfc/lpfc_attr.c: In function
Arnd> 'lpfc_enable_npiv_init': drivers/scsi/lpfc/lpfc_attr.c:2786:77:
Arnd> error: comparison of unsigned expression >= 0 is always true
Arnd> [-Werror=type-limits] drivers/scsi/lpfc/lpfc_attr.c: In function
Arnd> 'lpfc_enable_rrq_init': drivers/scsi/lpfc/lpfc_attr.c:2802:76:
Arnd> error: comparison of unsigned expression >= 0 is always true
Arnd> [-Werror=type-limits] drivers/scsi/lpfc/lpfc_attr.c: In function
Arnd> 'lpfc_suppress_link_up_init':
Arnd> drivers/scsi/lpfc/lpfc_attr.c:2812:2050: error: comparison of
Arnd> unsigned expression >= 0 is always true [-Werror=type-limits]
Arnd> drivers/scsi/lpfc/lpfc_attr.c: In function
Arnd> 'lpfc_log_verbose_init': drivers/scsi/lpfc/lpfc_attr.c:3064:1930:
Arnd> error: comparison of unsigned expression >= 0 is always true
Arnd> [-Werror=type-limits]

James, Dick: Please review!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning
  2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
                   ` (2 preceding siblings ...)
  2016-07-14  3:15 ` Martin K. Petersen
@ 2016-07-15 19:16 ` James Smart
  2016-07-20 23:54 ` Martin K. Petersen
  4 siblings, 0 replies; 10+ messages in thread
From: James Smart @ 2016-07-15 19:16 UTC (permalink / raw)
  To: Arnd Bergmann, James E.J. Bottomley, Martin K. Petersen
  Cc: James Smart, Dick Kennedy, Hannes Reinicke, James Bottomley,
	linux-scsi, linux-kernel

Patch is good.

Thanks

-- james


Signed-off-by: James Smart <james.smart@broadcom.com>

On 6/15/2016 1:42 PM, Arnd Bergmann wrote:
> When building with -Wextra, we get a lot of warnings for the lpfc driver
> concerning expressions that are always true, starting with:
>
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_npiv_init':
> drivers/scsi/lpfc/lpfc_attr.c:2786:77: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_enable_rrq_init':
> drivers/scsi/lpfc/lpfc_attr.c:2802:76: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_suppress_link_up_init':
> drivers/scsi/lpfc/lpfc_attr.c:2812:2050: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
> drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_log_verbose_init':
> drivers/scsi/lpfc/lpfc_attr.c:3064:1930: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>
> The code works as intented, but it would be nice to shut up the
> warning so we don't clutter up build logs with this. Using a
> separate inline function for it makes it clear to the compiler
> that the comparison is necessary in the caller but still lets
> it do the constant-folding.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/scsi/lpfc/lpfc_attr.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
> index cfec2eca4dd3..3e1d2e669902 100644
> --- a/drivers/scsi/lpfc/lpfc_attr.c
> +++ b/drivers/scsi/lpfc/lpfc_attr.c
> @@ -1620,6 +1620,11 @@ lpfc_sriov_hw_max_virtfn_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d\n", max_nr_virtfn);
>   }
>   
> +static inline bool lpfc_rangecheck(uint val, uint min, uint max)
> +{
> +	return val >= min && val <= max;
> +}
> +
>   /**
>    * lpfc_param_show - Return a cfg attribute value in decimal
>    *
> @@ -1697,7 +1702,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \
>   static int \
>   lpfc_##attr##_init(struct lpfc_hba *phba, uint val) \
>   { \
> -	if (val >= minval && val <= maxval) {\
> +	if (lpfc_rangecheck(val, minval, maxval)) {\
>   		phba->cfg_##attr = val;\
>   		return 0;\
>   	}\
> @@ -1732,7 +1737,7 @@ lpfc_##attr##_init(struct lpfc_hba *phba, uint val) \
>   static int \
>   lpfc_##attr##_set(struct lpfc_hba *phba, uint val) \
>   { \
> -	if (val >= minval && val <= maxval) {\
> +	if (lpfc_rangecheck(val, minval, maxval)) {\
>   		lpfc_printf_log(phba, KERN_ERR, LOG_INIT, \
>   			"3052 lpfc_" #attr " changed from %d to %d\n", \
>   			phba->cfg_##attr, val); \
> @@ -1856,7 +1861,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \
>   static int \
>   lpfc_##attr##_init(struct lpfc_vport *vport, uint val) \
>   { \
> -	if (val >= minval && val <= maxval) {\
> +	if (lpfc_rangecheck(val, minval, maxval)) {\
>   		vport->cfg_##attr = val;\
>   		return 0;\
>   	}\
> @@ -1888,7 +1893,7 @@ lpfc_##attr##_init(struct lpfc_vport *vport, uint val) \
>   static int \
>   lpfc_##attr##_set(struct lpfc_vport *vport, uint val) \
>   { \
> -	if (val >= minval && val <= maxval) {\
> +	if (lpfc_rangecheck(val, minval, maxval)) {\
>   		lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT, \
>   			"3053 lpfc_" #attr \
>   			" changed from %d (x%x) to %d (x%x)\n", \

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning
  2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
                   ` (3 preceding siblings ...)
  2016-07-15 19:16 ` James Smart
@ 2016-07-20 23:54 ` Martin K. Petersen
  4 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2016-07-20 23:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Hannes Reinicke, James Bottomley, linux-scsi,
	linux-kernel

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> When building with -Wextra, we get a lot of warnings for the lpfc
Arnd> driver concerning expressions that are always true, starting with:

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-21  0:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15 20:42 [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Arnd Bergmann
2016-06-15 20:54 ` [PATCH 2/2] scsi: wd7000: print sector number as 64-bit Arnd Bergmann
2016-06-16 17:36   ` kbuild test robot
2016-06-17 11:45     ` Arnd Bergmann
2016-06-21  1:12   ` Martin K. Petersen
2016-06-21 11:41     ` Arnd Bergmann
2016-06-16  7:39 ` [PATCH 1/2] scsi: lpfc: avoid harmless comparison warning Johannes Thumshirn
2016-07-14  3:15 ` Martin K. Petersen
2016-07-15 19:16 ` James Smart
2016-07-20 23:54 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).