* [PATCH] char: pcmcia: remove set but not used variable 'tmp'
@ 2021-05-14 6:21 Yu Kuai
2021-05-14 6:28 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Yu Kuai @ 2021-05-14 6:21 UTC (permalink / raw)
To: laforge, arnd, gregkh, akpm; +Cc: linux-kernel, yukuai3, yi.zhang
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/char/pcmcia/cm4000_cs.c:1053:16: warning: variable ‘tmp’
set but not used [-Wunused-but-set-variable]
It is never used and so can be removed.
Fixes: c1986ee9bea3 ("[PATCH] New Omnikey Cardman 4000 driver")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/char/pcmcia/cm4000_cs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 89681f07bc78..9ec25735425d 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -1050,7 +1050,6 @@ static ssize_t cmm_write(struct file *filp, const char __user *buf,
struct cm4000_dev *dev = filp->private_data;
unsigned int iobase = dev->p_dev->resource[0]->start;
unsigned short s;
- unsigned char tmp;
unsigned char infolen;
unsigned char sendT0;
unsigned short nsend;
@@ -1148,7 +1147,7 @@ static ssize_t cmm_write(struct file *filp, const char __user *buf,
set_cardparameter(dev);
/* dummy read, reset flag procedure received */
- tmp = inb(REG_FLAGS1(iobase));
+ inb(REG_FLAGS1(iobase));
dev->flags1 = 0x20 /* T_Active */
| (sendT0)
--
2.25.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] char: pcmcia: remove set but not used variable 'tmp'
2021-05-14 6:21 [PATCH] char: pcmcia: remove set but not used variable 'tmp' Yu Kuai
@ 2021-05-14 6:28 ` Arnd Bergmann
2021-05-15 7:15 ` yukuai (C)
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-05-14 6:28 UTC (permalink / raw)
To: Yu Kuai
Cc: Harald Welte, gregkh, akpm, Linux Kernel Mailing List, zhangyi (F)
On Fri, May 14, 2021 at 8:21 AM Yu Kuai <yukuai3@huawei.com> wrote:
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/char/pcmcia/cm4000_cs.c:1053:16: warning: variable ‘tmp’
> set but not used [-Wunused-but-set-variable]
>
> It is never used and so can be removed.
>
> Fixes: c1986ee9bea3 ("[PATCH] New Omnikey Cardman 4000 driver")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Looks good to me. This was likely written that way at a time when some
architecture implemented inb() as a macro, and ignoring its value
would cause a different warning.
Since you are already touching this file, can you have a look at this
warning as well:
drivers/char/pcmcia/cm4000_cs.c: In function 'set_protocol':
>> drivers/char/pcmcia/cm4000_cs.c:569:16: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
569 | pts_reply[i] = inb(REG_BUF_DATA(iobase));
| ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/char/pcmcia/cm4000_cs.c:567:2: note: within this loop
567 | for (i = 0; i < num_bytes_read; i++) {
This looks like a preexisting problem that was uncovered by a patch
that is now in linux-next to change the inb() definition once more,
I got a report from the kernel build bot about it after I merged the
patch into the asm-generic tree. It needs a range check on
num_bytes_read, or a Kconfig check to ensure it is not built on
architectures without working inb()/outb() operations.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] char: pcmcia: remove set but not used variable 'tmp'
2021-05-14 6:28 ` Arnd Bergmann
@ 2021-05-15 7:15 ` yukuai (C)
2021-05-15 9:05 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: yukuai (C) @ 2021-05-15 7:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Harald Welte, gregkh, akpm, Linux Kernel Mailing List, zhangyi (F)
On 2021/05/14 14:28, Arnd Bergmann wrote:
> On Fri, May 14, 2021 at 8:21 AM Yu Kuai <yukuai3@huawei.com> wrote:
>>
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> drivers/char/pcmcia/cm4000_cs.c:1053:16: warning: variable ‘tmp’
>> set but not used [-Wunused-but-set-variable]
>>
>> It is never used and so can be removed.
>>
>> Fixes: c1986ee9bea3 ("[PATCH] New Omnikey Cardman 4000 driver")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>
> Looks good to me. This was likely written that way at a time when some
> architecture implemented inb() as a macro, and ignoring its value
> would cause a different warning.
>
> Since you are already touching this file, can you have a look at this
> warning as well:
>
> drivers/char/pcmcia/cm4000_cs.c: In function 'set_protocol':
>>> drivers/char/pcmcia/cm4000_cs.c:569:16: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
> 569 | pts_reply[i] = inb(REG_BUF_DATA(iobase));
> | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/char/pcmcia/cm4000_cs.c:567:2: note: within this loop
> 567 | for (i = 0; i < num_bytes_read; i++) {
>
> This looks like a preexisting problem that was uncovered by a patch
> that is now in linux-next to change the inb() definition once more,
> I got a report from the kernel build bot about it after I merged the
> patch into the asm-generic tree. It needs a range check on
> num_bytes_read, or a Kconfig check to ensure it is not built on
> architectures without working inb()/outb() operations.
>
> Arnd
> .
>
Hi,
I'm not familar with the logical here, however, if io_read_num_rec_bytes
may get 'num_bytes_read' greater than 4, this loop will cause index out
of boundary. It make sense to me to add a range check. Futhermore, since
'num_bytes_read' is ensure to >= 4,I think we can change the loop to:
for (i = 0; i < 4; ++i) {
xoutb(i, REG_BUF_ADDR(iobase));
pts_reply[i] = inb(REG_BUF_DATA(iobase));
}
Thanks
Yu Kuai
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] char: pcmcia: remove set but not used variable 'tmp'
2021-05-15 7:15 ` yukuai (C)
@ 2021-05-15 9:05 ` Arnd Bergmann
0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2021-05-15 9:05 UTC (permalink / raw)
To: yukuai (C)
Cc: Harald Welte, gregkh, akpm, Linux Kernel Mailing List, zhangyi (F)
On Sat, May 15, 2021 at 9:15 AM yukuai (C) <yukuai3@huawei.com> wrote:
> On 2021/05/14 14:28, Arnd Bergmann wrote:
>
> I'm not familar with the logical here, however, if io_read_num_rec_bytes
> may get 'num_bytes_read' greater than 4, this loop will cause index out
> of boundary. It make sense to me to add a range check. Futhermore, since
> 'num_bytes_read' is ensure to >= 4,I think we can change the loop to:
>
> for (i = 0; i < 4; ++i) {
> xoutb(i, REG_BUF_ADDR(iobase));
> pts_reply[i] = inb(REG_BUF_DATA(iobase));
> }
Obviously it still has to use the num_bytes_read value to determine
how many bytes were actually received, but there should be an
added cap to tell the compiler that this is never more than 4.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-15 9:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 6:21 [PATCH] char: pcmcia: remove set but not used variable 'tmp' Yu Kuai
2021-05-14 6:28 ` Arnd Bergmann
2021-05-15 7:15 ` yukuai (C)
2021-05-15 9:05 ` Arnd Bergmann
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).