* [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 related [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 related [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.