| Message ID | 20241110125814.1899076-1-karprzy7@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*() | expand |
Hi Karol, kernel test robot noticed the following build errors: [auto build test ERROR on linuxtv-media-stage/master] [also build test ERROR on linus/master v6.12-rc6 next-20241108] [cannot apply to media-tree/master sailus-media-tree/streams sailus-media-tree/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Karol-Przybylski/media-cec-extron-da-hd-4k-plus-add-return-check-for-wait_for_completion/20241110-210018 base: https://git.linuxtv.org/media_stage.git master patch link: https://lore.kernel.org/r/20241110125814.1899076-1-karprzy7%40gmail.com patch subject: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*() config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241110/202411102341.VuOHMHGh-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241110/202411102341.VuOHMHGh-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411102341.VuOHMHGh-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c: In function 'extron_read_edid': >> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:17: error: 'ret' undeclared (first use in this function); did you mean 'net'? 563 | ret = wait_for_completion_killable_timeout(&extron->edid_completion, | ^~~ | net drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:17: note: each undeclared identifier is reported only once for each function it appears in vim +563 drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c 540 541 static void extron_read_edid(struct extron_port *port) 542 { 543 struct extron *extron = port->extron; 544 char cmd[10], reply[10]; 545 unsigned int idx; 546 547 idx = port->port.port + (port->is_input ? 0 : extron->num_in_ports); 548 snprintf(cmd, sizeof(cmd), "WR%uEDID", idx); 549 snprintf(reply, sizeof(reply), "EdidR%u", idx); 550 if (mutex_lock_interruptible(&extron->edid_lock)) 551 return; 552 if (port->read_edid) 553 goto unlock; 554 extron->edid_bytes_read = 0; 555 extron->edid_port = port; 556 port->edid_blocks = 0; 557 if (!port->has_edid) 558 goto no_edid; 559 560 extron->edid_reading = true; 561 562 if (!extron_send_and_wait(extron, port, cmd, reply)) { > 563 ret = wait_for_completion_killable_timeout(&extron->edid_completion, 564 msecs_to_jiffies(1000)); 565 if (ret < 0) 566 goto unlock; 567 } 568 if (port->edid_blocks) { 569 extron_parse_edid(port); 570 port->read_edid = true; 571 if (!port->is_input) 572 v4l2_ctrl_s_ctrl(port->ctrl_tx_edid_present, 1); 573 } 574 no_edid: 575 extron->edid_reading = false; 576 unlock: 577 mutex_unlock(&extron->edid_lock); 578 cancel_delayed_work_sync(&extron->work_update_edid); 579 if (manufacturer_name[0]) 580 schedule_delayed_work(&extron->work_update_edid, 581 msecs_to_jiffies(1000)); 582 } 583
Hi Karol,
kernel test robot noticed the following build errors:
[auto build test ERROR on linuxtv-media-stage/master]
[also build test ERROR on linus/master v6.12-rc6 next-20241108]
[cannot apply to media-tree/master sailus-media-tree/streams sailus-media-tree/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Karol-Przybylski/media-cec-extron-da-hd-4k-plus-add-return-check-for-wait_for_completion/20241110-210018
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20241110125814.1899076-1-karprzy7%40gmail.com
patch subject: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241111/202411111022.sX8ZCJZ8-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241111/202411111022.sX8ZCJZ8-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411111022.sX8ZCJZ8-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:29:
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h:13:
In file included from include/media/cec.h:19:
In file included from include/media/rc-core.h:13:
In file included from include/linux/kfifo.h:40:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:3: error: use of undeclared identifier 'ret'
563 | ret = wait_for_completion_killable_timeout(&extron->edid_completion,
| ^
drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:565:7: error: use of undeclared identifier 'ret'
565 | if (ret < 0)
| ^
7 warnings and 2 errors generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for MODVERSIONS
Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
Selected by [y]:
- RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
vim +/ret +563 drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
540
541 static void extron_read_edid(struct extron_port *port)
542 {
543 struct extron *extron = port->extron;
544 char cmd[10], reply[10];
545 unsigned int idx;
546
547 idx = port->port.port + (port->is_input ? 0 : extron->num_in_ports);
548 snprintf(cmd, sizeof(cmd), "WR%uEDID", idx);
549 snprintf(reply, sizeof(reply), "EdidR%u", idx);
550 if (mutex_lock_interruptible(&extron->edid_lock))
551 return;
552 if (port->read_edid)
553 goto unlock;
554 extron->edid_bytes_read = 0;
555 extron->edid_port = port;
556 port->edid_blocks = 0;
557 if (!port->has_edid)
558 goto no_edid;
559
560 extron->edid_reading = true;
561
562 if (!extron_send_and_wait(extron, port, cmd, reply)) {
> 563 ret = wait_for_completion_killable_timeout(&extron->edid_completion,
564 msecs_to_jiffies(1000));
565 if (ret < 0)
566 goto unlock;
567 }
568 if (port->edid_blocks) {
569 extron_parse_edid(port);
570 port->read_edid = true;
571 if (!port->is_input)
572 v4l2_ctrl_s_ctrl(port->ctrl_tx_edid_present, 1);
573 }
574 no_edid:
575 extron->edid_reading = false;
576 unlock:
577 mutex_unlock(&extron->edid_lock);
578 cancel_delayed_work_sync(&extron->work_update_edid);
579 if (manufacturer_name[0])
580 schedule_delayed_work(&extron->work_update_edid,
581 msecs_to_jiffies(1000));
582 }
583
On 10/11/2024 13:58, Karol Przybylski wrote: > According to scheduler/completion.rst, return status of wait_for_completion*() > function variants should be checked or be accompanied by explanation. > > I examined code in extron-da-hd-4k-plus.c and it does look like the return value > should be checked, but perhaps there is a reason for ignoring it. There is no need to check the return code in this specific case. If the wait was successful, then port->edid_blocks will be set, so effectively that 'if' is the result check of the wait_for_completion call. Regards, Hans > I drafted a patch for this, but I'm not entirely sure how to approach error handling in this case. > > Discovered in coverity, CID1599679 > > Signed-off-by: Karol Przybylski <karprzy7@gmail.com> > --- > .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > index cfbfc4c1b2e6..83a790117411 100644 > --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > @@ -559,9 +559,12 @@ static void extron_read_edid(struct extron_port *port) > > extron->edid_reading = true; > > - if (!extron_send_and_wait(extron, port, cmd, reply)) > - wait_for_completion_killable_timeout(&extron->edid_completion, > + if (!extron_send_and_wait(extron, port, cmd, reply)) { > + ret = wait_for_completion_killable_timeout(&extron->edid_completion, > msecs_to_jiffies(1000)); > + if (ret < 0) > + goto unlock; > + } > if (port->edid_blocks) { > extron_parse_edid(port); > port->read_edid = true;
diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c index cfbfc4c1b2e6..83a790117411 100644 --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c @@ -559,9 +559,12 @@ static void extron_read_edid(struct extron_port *port) extron->edid_reading = true; - if (!extron_send_and_wait(extron, port, cmd, reply)) - wait_for_completion_killable_timeout(&extron->edid_completion, + if (!extron_send_and_wait(extron, port, cmd, reply)) { + ret = wait_for_completion_killable_timeout(&extron->edid_completion, msecs_to_jiffies(1000)); + if (ret < 0) + goto unlock; + } if (port->edid_blocks) { extron_parse_edid(port); port->read_edid = true;
According to scheduler/completion.rst, return status of wait_for_completion*() function variants should be checked or be accompanied by explanation. I examined code in extron-da-hd-4k-plus.c and it does look like the return value should be checked, but perhaps there is a reason for ignoring it. I drafted a patch for this, but I'm not entirely sure how to approach error handling in this case. Discovered in coverity, CID1599679 Signed-off-by: Karol Przybylski <karprzy7@gmail.com> --- .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)