mbox series

[00/35] media: Fix coccinelle warning/errors

Message ID 20240415-fix-cocci-v1-0-477afb23728b@chromium.org
Headers show
Series media: Fix coccinelle warning/errors | expand

Message

Ricardo Ribalda April 15, 2024, 7:34 p.m. UTC
After this set is applied, these are the only warnings left:
drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)

CI tested:
https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (35):
      media: pci: mgb4: Refactor struct resources
      media: stb0899: Remove unreacheable code
      media: uvcvideo: Refactor iterators
      media: uvcvideo: Use max() macro
      media: go7007: Use min and max macros
      media: stm32-dcmipp: Remove redundant printk
      media: staging: sun6i-isp: Remove redundant printk
      media: dvb-frontends: tda18271c2dd: Remove casting during div
      media: v4l: async: refactor v4l2_async_create_ancillary_links
      staging: media: tegra-video: Use swap macro
      media: s2255: Use refcount_t instead of atomic_t for num_channels
      media: platform: mtk-mdp3: Use refcount_t for job_count
      media: common: saa7146: Use min macro
      media: dvb-frontends: drx39xyj: Use min macro
      media: netup_unidvb: Use min macro
      media: au0828: Use min macro
      media: flexcop-usb: Use min macro
      media: gspca: cpia1: Use min macro
      media: stk1160: Use min macro
      media: tegra-vde: Refactor timeout handling
      media: venus: Use div64_u64
      media: i2c: st-mipid02: Use the correct div function
      media: dvb-frontends: tda10048: Use the right div
      media: tc358746: Use the correct div_ function
      media: venus: Use the correct div_ function
      media: venus: Refator return path
      media: dib0700: Refator return path
      media: usb: cx231xx: Refator return path
      media: i2c: rdacm20: Refator return path
      media: i2c: et8ek8: Refator return path
      media: cx231xx: Refator return path
      media: si4713: Refator return path
      media: ttpci: Refator return path
      media: hdpvr: Refator return path
      media: venus: Refator return path

 drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
 drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
 drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
 drivers/media/dvb-frontends/tda10048.c             |  3 +--
 drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
 drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
 drivers/media/i2c/rdacm20.c                        |  5 ++++-
 drivers/media/i2c/st-mipid02.c                     |  2 +-
 drivers/media/i2c/tc358746.c                       |  3 +--
 drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
 drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
 drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
 drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
 .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
 .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
 .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
 .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
 drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
 drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
 drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
 .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
 drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
 drivers/media/usb/au0828/au0828-video.c            |  5 +----
 drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
 drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
 drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
 drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
 drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
 drivers/media/usb/gspca/cpia1.c                    |  6 ++---
 drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
 drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
 drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
 drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
 drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
 drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
 36 files changed, 132 insertions(+), 129 deletions(-)
---
base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
change-id: 20240415-fix-cocci-2df3ef22a6f7

Best regards,

Comments

Laurent Pinchart April 15, 2024, 7:53 p.m. UTC | #1
Hi Ricardo,

I'm afraid I won't have time to review any of this for the time being.
Unless you would like me to put uvcvideo reviews on hold ;-)

Jokes aside, my first reaction was that this feels like a bit of a waste
of maintainer's time :-S

On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> After this set is applied, these are the only warnings left:
> drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> 
> CI tested:
> https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Ricardo Ribalda (35):
>       media: pci: mgb4: Refactor struct resources
>       media: stb0899: Remove unreacheable code
>       media: uvcvideo: Refactor iterators
>       media: uvcvideo: Use max() macro
>       media: go7007: Use min and max macros
>       media: stm32-dcmipp: Remove redundant printk
>       media: staging: sun6i-isp: Remove redundant printk
>       media: dvb-frontends: tda18271c2dd: Remove casting during div
>       media: v4l: async: refactor v4l2_async_create_ancillary_links
>       staging: media: tegra-video: Use swap macro
>       media: s2255: Use refcount_t instead of atomic_t for num_channels
>       media: platform: mtk-mdp3: Use refcount_t for job_count
>       media: common: saa7146: Use min macro
>       media: dvb-frontends: drx39xyj: Use min macro
>       media: netup_unidvb: Use min macro
>       media: au0828: Use min macro
>       media: flexcop-usb: Use min macro
>       media: gspca: cpia1: Use min macro
>       media: stk1160: Use min macro
>       media: tegra-vde: Refactor timeout handling
>       media: venus: Use div64_u64
>       media: i2c: st-mipid02: Use the correct div function
>       media: dvb-frontends: tda10048: Use the right div
>       media: tc358746: Use the correct div_ function
>       media: venus: Use the correct div_ function
>       media: venus: Refator return path
>       media: dib0700: Refator return path
>       media: usb: cx231xx: Refator return path
>       media: i2c: rdacm20: Refator return path
>       media: i2c: et8ek8: Refator return path
>       media: cx231xx: Refator return path
>       media: si4713: Refator return path
>       media: ttpci: Refator return path
>       media: hdpvr: Refator return path
>       media: venus: Refator return path
> 
>  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
>  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
>  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
>  drivers/media/dvb-frontends/tda10048.c             |  3 +--
>  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
>  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
>  drivers/media/i2c/rdacm20.c                        |  5 ++++-
>  drivers/media/i2c/st-mipid02.c                     |  2 +-
>  drivers/media/i2c/tc358746.c                       |  3 +--
>  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
>  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
>  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
>  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
>  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
>  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
>  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
>  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
>  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
>  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
>  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
>  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
>  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
>  drivers/media/usb/au0828/au0828-video.c            |  5 +----
>  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
>  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
>  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
>  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
>  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
>  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
>  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
>  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
>  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
>  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
>  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
>  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
>  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
>  36 files changed, 132 insertions(+), 129 deletions(-)
> ---
> base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> change-id: 20240415-fix-cocci-2df3ef22a6f7
> 
> Best regards,
Ricardo Ribalda April 15, 2024, 7:58 p.m. UTC | #2
Hi Laurent

On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> I'm afraid I won't have time to review any of this for the time being.
> Unless you would like me to put uvcvideo reviews on hold ;-)
>
> Jokes aside, my first reaction was that this feels like a bit of a waste
> of maintainer's time :-S

This is part of the media-ci effort.

It is definitely not the most fun patches to do or review, but someone
has to do it :)

The whole idea is that we want to get as little warnings as possible
from the static analysers, after this patchset we almost achieve that.

It is only 2 trivial uvc patches, I can ask someone from my team to
review it if you want and trust them ;)

Regards!

>
> On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > After this set is applied, these are the only warnings left:
> > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> >
> > CI tested:
> > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > Ricardo Ribalda (35):
> >       media: pci: mgb4: Refactor struct resources
> >       media: stb0899: Remove unreacheable code
> >       media: uvcvideo: Refactor iterators
> >       media: uvcvideo: Use max() macro
> >       media: go7007: Use min and max macros
> >       media: stm32-dcmipp: Remove redundant printk
> >       media: staging: sun6i-isp: Remove redundant printk
> >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> >       staging: media: tegra-video: Use swap macro
> >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> >       media: platform: mtk-mdp3: Use refcount_t for job_count
> >       media: common: saa7146: Use min macro
> >       media: dvb-frontends: drx39xyj: Use min macro
> >       media: netup_unidvb: Use min macro
> >       media: au0828: Use min macro
> >       media: flexcop-usb: Use min macro
> >       media: gspca: cpia1: Use min macro
> >       media: stk1160: Use min macro
> >       media: tegra-vde: Refactor timeout handling
> >       media: venus: Use div64_u64
> >       media: i2c: st-mipid02: Use the correct div function
> >       media: dvb-frontends: tda10048: Use the right div
> >       media: tc358746: Use the correct div_ function
> >       media: venus: Use the correct div_ function
> >       media: venus: Refator return path
> >       media: dib0700: Refator return path
> >       media: usb: cx231xx: Refator return path
> >       media: i2c: rdacm20: Refator return path
> >       media: i2c: et8ek8: Refator return path
> >       media: cx231xx: Refator return path
> >       media: si4713: Refator return path
> >       media: ttpci: Refator return path
> >       media: hdpvr: Refator return path
> >       media: venus: Refator return path
> >
> >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> >  drivers/media/i2c/tc358746.c                       |  3 +--
> >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> >  36 files changed, 132 insertions(+), 129 deletions(-)
> > ---
> > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > change-id: 20240415-fix-cocci-2df3ef22a6f7
> >
> > Best regards,
>
> --
> Regards,
>
> Laurent Pinchart
Niklas Söderlund April 15, 2024, 8:33 p.m. UTC | #3
Hi Ricardo,

Thanks for cleaning up.

On 2024-04-15 21:58:58 +0200, Ricardo Ribalda wrote:
> Hi Laurent
> 
> On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Ricardo,
> >
> > I'm afraid I won't have time to review any of this for the time being.
> > Unless you would like me to put uvcvideo reviews on hold ;-)
> >
> > Jokes aside, my first reaction was that this feels like a bit of a waste
> > of maintainer's time :-S
> 
> This is part of the media-ci effort.
> 
> It is definitely not the most fun patches to do or review, but someone
> has to do it :)
> 
> The whole idea is that we want to get as little warnings as possible
> from the static analysers, after this patchset we almost achieve that.

I understand and I think it's a good goal to aim for zero warnings. But 
some of the fixes here are IMHO not helpful, for example I find this 
type of change counter productive.

-       return ret < 0 ? ret : 0;
+
+       if (ret < 0)
+               return ret;
+       return 0;

Maybe it's better to disable this type of checks in the linter?

> 
> It is only 2 trivial uvc patches, I can ask someone from my team to
> review it if you want and trust them ;)
> 
> Regards!
> 
> >
> > On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > > After this set is applied, these are the only warnings left:
> > > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > >
> > > CI tested:
> > > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > Ricardo Ribalda (35):
> > >       media: pci: mgb4: Refactor struct resources
> > >       media: stb0899: Remove unreacheable code
> > >       media: uvcvideo: Refactor iterators
> > >       media: uvcvideo: Use max() macro
> > >       media: go7007: Use min and max macros
> > >       media: stm32-dcmipp: Remove redundant printk
> > >       media: staging: sun6i-isp: Remove redundant printk
> > >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> > >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> > >       staging: media: tegra-video: Use swap macro
> > >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> > >       media: platform: mtk-mdp3: Use refcount_t for job_count
> > >       media: common: saa7146: Use min macro
> > >       media: dvb-frontends: drx39xyj: Use min macro
> > >       media: netup_unidvb: Use min macro
> > >       media: au0828: Use min macro
> > >       media: flexcop-usb: Use min macro
> > >       media: gspca: cpia1: Use min macro
> > >       media: stk1160: Use min macro
> > >       media: tegra-vde: Refactor timeout handling
> > >       media: venus: Use div64_u64
> > >       media: i2c: st-mipid02: Use the correct div function
> > >       media: dvb-frontends: tda10048: Use the right div
> > >       media: tc358746: Use the correct div_ function
> > >       media: venus: Use the correct div_ function
> > >       media: venus: Refator return path
> > >       media: dib0700: Refator return path
> > >       media: usb: cx231xx: Refator return path
> > >       media: i2c: rdacm20: Refator return path
> > >       media: i2c: et8ek8: Refator return path
> > >       media: cx231xx: Refator return path
> > >       media: si4713: Refator return path
> > >       media: ttpci: Refator return path
> > >       media: hdpvr: Refator return path
> > >       media: venus: Refator return path
> > >
> > >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> > >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> > >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> > >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> > >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> > >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> > >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> > >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> > >  drivers/media/i2c/tc358746.c                       |  3 +--
> > >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> > >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> > >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> > >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> > >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> > >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> > >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> > >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> > >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> > >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> > >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> > >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> > >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> > >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> > >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> > >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> > >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> > >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> > >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> > >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> > >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> > >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> > >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> > >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> > >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> > >  36 files changed, 132 insertions(+), 129 deletions(-)
> > > ---
> > > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > > change-id: 20240415-fix-cocci-2df3ef22a6f7
> > >
> > > Best regards,
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> 
> 
> 
> -- 
> Ricardo Ribalda
Ricardo Ribalda April 15, 2024, 9:16 p.m. UTC | #4
Hi Niklas

On Mon, 15 Apr 2024 at 22:33, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
>
> Hi Ricardo,
>
> Thanks for cleaning up.
>
> On 2024-04-15 21:58:58 +0200, Ricardo Ribalda wrote:
> > Hi Laurent
> >
> > On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > I'm afraid I won't have time to review any of this for the time being.
> > > Unless you would like me to put uvcvideo reviews on hold ;-)
> > >
> > > Jokes aside, my first reaction was that this feels like a bit of a waste
> > > of maintainer's time :-S
> >
> > This is part of the media-ci effort.
> >
> > It is definitely not the most fun patches to do or review, but someone
> > has to do it :)
> >
> > The whole idea is that we want to get as little warnings as possible
> > from the static analysers, after this patchset we almost achieve that.
>
> I understand and I think it's a good goal to aim for zero warnings. But
> some of the fixes here are IMHO not helpful, for example I find this
> type of change counter productive.
>
> -       return ret < 0 ? ret : 0;
> +
> +       if (ret < 0)
> +               return ret;
> +       return 0;

I was on the edge on those ones as well...

Maybe we can try to fix the .cocci file to ignore that pattern?
https://lore.kernel.org/lkml/20240415-minimax-v1-1-5feb20d66a79@chromium.org/T/#u

Regards!





>
> Maybe it's better to disable this type of checks in the linter?
>
> >
> > It is only 2 trivial uvc patches, I can ask someone from my team to
> > review it if you want and trust them ;)
> >
> > Regards!
> >
> > >
> > > On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > > > After this set is applied, these are the only warnings left:
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > > > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > > > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > > > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > > > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > > > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > >
> > > > CI tested:
> > > > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > > Ricardo Ribalda (35):
> > > >       media: pci: mgb4: Refactor struct resources
> > > >       media: stb0899: Remove unreacheable code
> > > >       media: uvcvideo: Refactor iterators
> > > >       media: uvcvideo: Use max() macro
> > > >       media: go7007: Use min and max macros
> > > >       media: stm32-dcmipp: Remove redundant printk
> > > >       media: staging: sun6i-isp: Remove redundant printk
> > > >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> > > >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> > > >       staging: media: tegra-video: Use swap macro
> > > >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> > > >       media: platform: mtk-mdp3: Use refcount_t for job_count
> > > >       media: common: saa7146: Use min macro
> > > >       media: dvb-frontends: drx39xyj: Use min macro
> > > >       media: netup_unidvb: Use min macro
> > > >       media: au0828: Use min macro
> > > >       media: flexcop-usb: Use min macro
> > > >       media: gspca: cpia1: Use min macro
> > > >       media: stk1160: Use min macro
> > > >       media: tegra-vde: Refactor timeout handling
> > > >       media: venus: Use div64_u64
> > > >       media: i2c: st-mipid02: Use the correct div function
> > > >       media: dvb-frontends: tda10048: Use the right div
> > > >       media: tc358746: Use the correct div_ function
> > > >       media: venus: Use the correct div_ function
> > > >       media: venus: Refator return path
> > > >       media: dib0700: Refator return path
> > > >       media: usb: cx231xx: Refator return path
> > > >       media: i2c: rdacm20: Refator return path
> > > >       media: i2c: et8ek8: Refator return path
> > > >       media: cx231xx: Refator return path
> > > >       media: si4713: Refator return path
> > > >       media: ttpci: Refator return path
> > > >       media: hdpvr: Refator return path
> > > >       media: venus: Refator return path
> > > >
> > > >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> > > >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> > > >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> > > >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> > > >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> > > >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> > > >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> > > >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> > > >  drivers/media/i2c/tc358746.c                       |  3 +--
> > > >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> > > >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> > > >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> > > >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> > > >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> > > >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> > > >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> > > >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> > > >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> > > >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> > > >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> > > >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> > > >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> > > >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> > > >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> > > >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> > > >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> > > >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> > > >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> > > >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> > > >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> > > >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> > > >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> > > >  36 files changed, 132 insertions(+), 129 deletions(-)
> > > > ---
> > > > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > > > change-id: 20240415-fix-cocci-2df3ef22a6f7
> > > >
> > > > Best regards,
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> >
> >
> >
> > --
> > Ricardo Ribalda
>
> --
> Kind Regards,
> Niklas Söderlund



--
Ricardo Ribalda
Niklas Söderlund April 15, 2024, 9:38 p.m. UTC | #5
Hi Ricardo,

On 2024-04-15 23:16:55 +0200, Ricardo Ribalda wrote:
> Hi Niklas
> 
> On Mon, 15 Apr 2024 at 22:33, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> >
> > Hi Ricardo,
> >
> > Thanks for cleaning up.
> >
> > On 2024-04-15 21:58:58 +0200, Ricardo Ribalda wrote:
> > > Hi Laurent
> > >
> > > On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com> wrote:
> > > >
> > > > Hi Ricardo,
> > > >
> > > > I'm afraid I won't have time to review any of this for the time being.
> > > > Unless you would like me to put uvcvideo reviews on hold ;-)
> > > >
> > > > Jokes aside, my first reaction was that this feels like a bit of a waste
> > > > of maintainer's time :-S
> > >
> > > This is part of the media-ci effort.
> > >
> > > It is definitely not the most fun patches to do or review, but someone
> > > has to do it :)
> > >
> > > The whole idea is that we want to get as little warnings as possible
> > > from the static analysers, after this patchset we almost achieve that.
> >
> > I understand and I think it's a good goal to aim for zero warnings. But
> > some of the fixes here are IMHO not helpful, for example I find this
> > type of change counter productive.
> >
> > -       return ret < 0 ? ret : 0;
> > +
> > +       if (ret < 0)
> > +               return ret;
> > +       return 0;
> 
> I was on the edge on those ones as well...
> 
> Maybe we can try to fix the .cocci file to ignore that pattern?
> https://lore.kernel.org/lkml/20240415-minimax-v1-1-5feb20d66a79@chromium.org/T/#u

Thanks for looking into it! I think this is a good idea.

> 
> Regards!
> 
> 
> 
> 
> 
> >
> > Maybe it's better to disable this type of checks in the linter?
> >
> > >
> > > It is only 2 trivial uvc patches, I can ask someone from my team to
> > > review it if you want and trust them ;)
> > >
> > > Regards!
> > >
> > > >
> > > > On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > > > > After this set is applied, these are the only warnings left:
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > > > > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > > > > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > > > > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > > > > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > > > > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > >
> > > > > CI tested:
> > > > > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > ---
> > > > > Ricardo Ribalda (35):
> > > > >       media: pci: mgb4: Refactor struct resources
> > > > >       media: stb0899: Remove unreacheable code
> > > > >       media: uvcvideo: Refactor iterators
> > > > >       media: uvcvideo: Use max() macro
> > > > >       media: go7007: Use min and max macros
> > > > >       media: stm32-dcmipp: Remove redundant printk
> > > > >       media: staging: sun6i-isp: Remove redundant printk
> > > > >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> > > > >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> > > > >       staging: media: tegra-video: Use swap macro
> > > > >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> > > > >       media: platform: mtk-mdp3: Use refcount_t for job_count
> > > > >       media: common: saa7146: Use min macro
> > > > >       media: dvb-frontends: drx39xyj: Use min macro
> > > > >       media: netup_unidvb: Use min macro
> > > > >       media: au0828: Use min macro
> > > > >       media: flexcop-usb: Use min macro
> > > > >       media: gspca: cpia1: Use min macro
> > > > >       media: stk1160: Use min macro
> > > > >       media: tegra-vde: Refactor timeout handling
> > > > >       media: venus: Use div64_u64
> > > > >       media: i2c: st-mipid02: Use the correct div function
> > > > >       media: dvb-frontends: tda10048: Use the right div
> > > > >       media: tc358746: Use the correct div_ function
> > > > >       media: venus: Use the correct div_ function
> > > > >       media: venus: Refator return path
> > > > >       media: dib0700: Refator return path
> > > > >       media: usb: cx231xx: Refator return path
> > > > >       media: i2c: rdacm20: Refator return path
> > > > >       media: i2c: et8ek8: Refator return path
> > > > >       media: cx231xx: Refator return path
> > > > >       media: si4713: Refator return path
> > > > >       media: ttpci: Refator return path
> > > > >       media: hdpvr: Refator return path
> > > > >       media: venus: Refator return path
> > > > >
> > > > >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> > > > >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> > > > >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> > > > >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> > > > >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> > > > >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> > > > >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> > > > >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> > > > >  drivers/media/i2c/tc358746.c                       |  3 +--
> > > > >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> > > > >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> > > > >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> > > > >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> > > > >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> > > > >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> > > > >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> > > > >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> > > > >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> > > > >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> > > > >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> > > > >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> > > > >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> > > > >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> > > > >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> > > > >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> > > > >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> > > > >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> > > > >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> > > > >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> > > > >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> > > > >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> > > > >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> > > > >  36 files changed, 132 insertions(+), 129 deletions(-)
> > > > > ---
> > > > > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > > > > change-id: 20240415-fix-cocci-2df3ef22a6f7
> > > > >
> > > > > Best regards,
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart
> > >
> > >
> > >
> > > --
> > > Ricardo Ribalda
> >
> > --
> > Kind Regards,
> > Niklas Söderlund
> 
> 
> 
> --
> Ricardo Ribalda
Sergey Senozhatsky April 16, 2024, 4:39 a.m. UTC | #6
On (24/04/15 19:34), Ricardo Ribalda wrote:
[..]
> @@ -2165,7 +2167,7 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
>  int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>  	struct uvc_xu_control_query *xqry)
>  {
> -	struct uvc_entity *entity;
> +	struct uvc_entity *entity, *iter;
>  	struct uvc_control *ctrl;
>  	unsigned int i;
>  	bool found;

Is `found` still used?

> @@ -2175,16 +2177,16 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>  	int ret;
>  
>  	/* Find the extension unit. */
> -	found = false;
> -	list_for_each_entry(entity, &chain->entities, chain) {
> -		if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT &&
> -		    entity->id == xqry->unit) {
> -			found = true;
> +	entity = NULL;
> +	list_for_each_entry(iter, &chain->entities, chain) {
> +		if (UVC_ENTITY_TYPE(iter) == UVC_VC_EXTENSION_UNIT &&
> +		    iter->id == xqry->unit) {
> +			entity = iter;
>  			break;
>  		}
>  	}
>  
> -	if (!found) {
> +	if (!entity) {
>  		uvc_dbg(chain->dev, CONTROL, "Extension unit %u not found\n",
>  			xqry->unit);
>  		return -ENOENT;
Sergey Senozhatsky April 16, 2024, 5:08 a.m. UTC | #7
On (24/04/16 13:39), Sergey Senozhatsky wrote:
> On (24/04/15 19:34), Ricardo Ribalda wrote:
> [..]
> > @@ -2165,7 +2167,7 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
> >  int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> >  	struct uvc_xu_control_query *xqry)
> >  {
> > -	struct uvc_entity *entity;
> > +	struct uvc_entity *entity, *iter;
> >  	struct uvc_control *ctrl;
> >  	unsigned int i;
> >  	bool found;
> 
> Is `found` still used?

It is. Never mind.

FWIW
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Laurent Pinchart April 16, 2024, 7:08 a.m. UTC | #8
On Mon, Apr 15, 2024 at 11:38:46PM +0200, Niklas Söderlund wrote:
> On 2024-04-15 23:16:55 +0200, Ricardo Ribalda wrote:
> > On Mon, 15 Apr 2024 at 22:33, Niklas Söderlund wrote:
> > > On 2024-04-15 21:58:58 +0200, Ricardo Ribalda wrote:
> > > > On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart wrote:
> > > > >
> > > > > Hi Ricardo,
> > > > >
> > > > > I'm afraid I won't have time to review any of this for the time being.
> > > > > Unless you would like me to put uvcvideo reviews on hold ;-)
> > > > >
> > > > > Jokes aside, my first reaction was that this feels like a bit of a waste
> > > > > of maintainer's time :-S
> > > >
> > > > This is part of the media-ci effort.

Ah. Mentioning that in the cover letter would have helped.

> > > > It is definitely not the most fun patches to do or review, but someone
> > > > has to do it :)
> > > >
> > > > The whole idea is that we want to get as little warnings as possible
> > > > from the static analysers, after this patchset we almost achieve that.
> > >
> > > I understand and I think it's a good goal to aim for zero warnings. But
> > > some of the fixes here are IMHO not helpful, for example I find this
> > > type of change counter productive.
> > >
> > > -       return ret < 0 ? ret : 0;
> > > +
> > > +       if (ret < 0)
> > > +               return ret;
> > > +       return 0;
> > 
> > I was on the edge on those ones as well...
> > 
> > Maybe we can try to fix the .cocci file to ignore that pattern?
> > https://lore.kernel.org/lkml/20240415-minimax-v1-1-5feb20d66a79@chromium.org/T/#u
> 
> Thanks for looking into it! I think this is a good idea.

I agree, this is the first type of change I saw in the series, and it
made me dispair for a moment :-)

> > > Maybe it's better to disable this type of checks in the linter?
> > >
> > > > It is only 2 trivial uvc patches, I can ask someone from my team to
> > > > review it if you want and trust them ;)
> > > >
> > > > Regards!
> > > >
> > > > > On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > > > > > After this set is applied, these are the only warnings left:
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > > > > > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > > > > > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > > > > > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > > > > > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > >
> > > > > > CI tested:
> > > > > > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> > > > > >
> > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > ---
> > > > > > Ricardo Ribalda (35):
> > > > > >       media: pci: mgb4: Refactor struct resources
> > > > > >       media: stb0899: Remove unreacheable code
> > > > > >       media: uvcvideo: Refactor iterators
> > > > > >       media: uvcvideo: Use max() macro
> > > > > >       media: go7007: Use min and max macros
> > > > > >       media: stm32-dcmipp: Remove redundant printk
> > > > > >       media: staging: sun6i-isp: Remove redundant printk
> > > > > >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> > > > > >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> > > > > >       staging: media: tegra-video: Use swap macro
> > > > > >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> > > > > >       media: platform: mtk-mdp3: Use refcount_t for job_count
> > > > > >       media: common: saa7146: Use min macro
> > > > > >       media: dvb-frontends: drx39xyj: Use min macro
> > > > > >       media: netup_unidvb: Use min macro
> > > > > >       media: au0828: Use min macro
> > > > > >       media: flexcop-usb: Use min macro
> > > > > >       media: gspca: cpia1: Use min macro
> > > > > >       media: stk1160: Use min macro
> > > > > >       media: tegra-vde: Refactor timeout handling
> > > > > >       media: venus: Use div64_u64
> > > > > >       media: i2c: st-mipid02: Use the correct div function
> > > > > >       media: dvb-frontends: tda10048: Use the right div
> > > > > >       media: tc358746: Use the correct div_ function
> > > > > >       media: venus: Use the correct div_ function
> > > > > >       media: venus: Refator return path
> > > > > >       media: dib0700: Refator return path
> > > > > >       media: usb: cx231xx: Refator return path
> > > > > >       media: i2c: rdacm20: Refator return path
> > > > > >       media: i2c: et8ek8: Refator return path
> > > > > >       media: cx231xx: Refator return path
> > > > > >       media: si4713: Refator return path
> > > > > >       media: ttpci: Refator return path
> > > > > >       media: hdpvr: Refator return path
> > > > > >       media: venus: Refator return path
> > > > > >
> > > > > >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> > > > > >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> > > > > >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> > > > > >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> > > > > >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> > > > > >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> > > > > >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> > > > > >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> > > > > >  drivers/media/i2c/tc358746.c                       |  3 +--
> > > > > >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> > > > > >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> > > > > >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> > > > > >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> > > > > >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> > > > > >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> > > > > >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> > > > > >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> > > > > >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> > > > > >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> > > > > >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> > > > > >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> > > > > >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> > > > > >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> > > > > >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> > > > > >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> > > > > >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> > > > > >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> > > > > >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> > > > > >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> > > > > >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> > > > > >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> > > > > >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> > > > > >  36 files changed, 132 insertions(+), 129 deletions(-)
> > > > > > ---
> > > > > > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > > > > > change-id: 20240415-fix-cocci-2df3ef22a6f7
> > > > > >
> > > > > > Best regards,
Dan Carpenter April 16, 2024, 7:37 a.m. UTC | #9
On Mon, Apr 15, 2024 at 07:34:24PM +0000, Ricardo Ribalda wrote:
> platform_get_irq() already prints an error for us.
> 
> Found by cocci:
> drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c:389:2-9: line 389 is redundant because platform_get_irq() already prints an error
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c
> index 5c0a45394cba..a6424fe7023b 100644
> --- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c
> +++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c
> @@ -386,7 +386,6 @@ static int sun6i_isp_resources_setup(struct sun6i_isp_device *isp_dev,
>  
>  	irq = platform_get_irq(platform_dev, 0);
>  	if (irq < 0) {
> -		dev_err(dev, "failed to get interrupt\n");
>  		ret = -ENXIO;

This is more fall out from when irq functions used to return zero (16
years ago).  Instead of ret = -ENXIO, set ret = irq.

regards,
dan carpenter
Dan Carpenter April 16, 2024, 7:44 a.m. UTC | #10
On Mon, Apr 15, 2024 at 07:34:26PM +0000, Ricardo Ribalda wrote:
> Return 0 without checking IS_ERR or PTR_ERR if CONFIG_MEDIA_CONTROLLER
> is not enabled.
> 
> This makes cocci happier:
> 
> drivers/media/v4l2-core/v4l2-async.c:331:23-30: ERROR: PTR_ERR applied after initialization to constant on line 319
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 4bb073587817..e26a011c89c4 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -316,9 +316,8 @@ v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier);
>  static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n,
>  					     struct v4l2_subdev *sd)
>  {
> -	struct media_link *link = NULL;
> -
>  #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
> +	struct media_link *link;
>  

I think another way you could write this is to remove the #ifs...

	struct media_link *link;

	if (!IS_ENABLED(CONFIG_MEDIA_CONTROLLER))
		return 0;

	if (sd->entity.function != MEDIA_ENT_F_LENS && ...

regards,
dan carpenter

>  	if (sd->entity.function != MEDIA_ENT_F_LENS &&
>  	    sd->entity.function != MEDIA_ENT_F_FLASH)
> @@ -326,9 +325,10 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n,
>  
>  	link = media_create_ancillary_link(&n->sd->entity, &sd->entity);
>  
> -#endif
> -
>  	return IS_ERR(link) ? PTR_ERR(link) : 0;
> +#else
> +	return 0;
> +#endif
>  }
>
Julia Lawall April 16, 2024, 7:59 a.m. UTC | #11
On Tue, 16 Apr 2024, Laurent Pinchart wrote:

> On Mon, Apr 15, 2024 at 11:38:46PM +0200, Niklas Söderlund wrote:
> > On 2024-04-15 23:16:55 +0200, Ricardo Ribalda wrote:
> > > On Mon, 15 Apr 2024 at 22:33, Niklas Söderlund wrote:
> > > > On 2024-04-15 21:58:58 +0200, Ricardo Ribalda wrote:
> > > > > On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart wrote:
> > > > > >
> > > > > > Hi Ricardo,
> > > > > >
> > > > > > I'm afraid I won't have time to review any of this for the time being.
> > > > > > Unless you would like me to put uvcvideo reviews on hold ;-)
> > > > > >
> > > > > > Jokes aside, my first reaction was that this feels like a bit of a waste
> > > > > > of maintainer's time :-S
> > > > >
> > > > > This is part of the media-ci effort.
>
> Ah. Mentioning that in the cover letter would have helped.
>
> > > > > It is definitely not the most fun patches to do or review, but someone
> > > > > has to do it :)
> > > > >
> > > > > The whole idea is that we want to get as little warnings as possible
> > > > > from the static analysers, after this patchset we almost achieve that.
> > > >
> > > > I understand and I think it's a good goal to aim for zero warnings. But
> > > > some of the fixes here are IMHO not helpful, for example I find this
> > > > type of change counter productive.
> > > >
> > > > -       return ret < 0 ? ret : 0;
> > > > +
> > > > +       if (ret < 0)
> > > > +               return ret;
> > > > +       return 0;
> > >
> > > I was on the edge on those ones as well...
> > >
> > > Maybe we can try to fix the .cocci file to ignore that pattern?
> > > https://lore.kernel.org/lkml/20240415-minimax-v1-1-5feb20d66a79@chromium.org/T/#u
> >
> > Thanks for looking into it! I think this is a good idea.
>
> I agree, this is the first type of change I saw in the series, and it
> made me dispair for a moment :-)
>
> > > > Maybe it's better to disable this type of checks in the linter?

I would be happy to get rid of it.  The person who made the semantic patch
originally expressed the opinion that maybe it could be useful sometimes,
but I always discard these patches when 0-day forwards them to me for
approval.

When it's not a 0 value, using min and max can often improve readability,
so I think it would be unfortunate to remove the semantic patch
completely.

julia


> > > >
> > > > > It is only 2 trivial uvc patches, I can ask someone from my team to
> > > > > review it if you want and trust them ;)
> > > > >
> > > > > Regards!
> > > > >
> > > > > > On Mon, Apr 15, 2024 at 07:34:17PM +0000, Ricardo Ribalda wrote:
> > > > > > > After this set is applied, these are the only warnings left:
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
> > > > > > > drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689
> > > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > > > > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
> > > > > > > drivers/media/dvb-frontends/stv090x.c:799:1-7: preceding lock on line 768
> > > > > > > drivers/media/usb/go7007/go7007-i2c.c:125:1-7: preceding lock on line 61
> > > > > > > drivers/media/rc/imon.c:1167:1-7: preceding lock on line 1153
> > > > > > > drivers/media/pci/cx18/cx18-scb.h:261:22-29: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:85:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:154:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:171:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:180:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:189:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:201:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:220:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_cmds.h:230:5-16: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:764:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1008:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1014:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1041:5-15: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1088:39-51: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1093:5-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1144:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1239:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1267:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/qcom/venus/hfi_helper.h:1272:4-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/common/siano/smscoreapi.h:619:5-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/common/siano/smscoreapi.h:669:6-13: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/common/siano/smscoreapi.h:1049:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/common/siano/smscoreapi.h:1055:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:171:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/dvb-frontends/mxl5xx_defs.h:182:4-8: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/allegro-dvt/nal-hevc.h:102:14-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/media/platform/xilinx/xilinx-dma.h:100:19-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > > drivers/staging/media/atomisp/pci/atomisp_tpg.h:30:18-22: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> > > > > > >
> > > > > > > CI tested:
> > > > > > > https://gitlab.freedesktop.org/linux-media/media-staging/-/commit/055b5211c68e721c3a7090be5373cf44859da1a7/pipelines?ref=ribalda%2Ftest-cocci
> > > > > > >
> > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > > ---
> > > > > > > Ricardo Ribalda (35):
> > > > > > >       media: pci: mgb4: Refactor struct resources
> > > > > > >       media: stb0899: Remove unreacheable code
> > > > > > >       media: uvcvideo: Refactor iterators
> > > > > > >       media: uvcvideo: Use max() macro
> > > > > > >       media: go7007: Use min and max macros
> > > > > > >       media: stm32-dcmipp: Remove redundant printk
> > > > > > >       media: staging: sun6i-isp: Remove redundant printk
> > > > > > >       media: dvb-frontends: tda18271c2dd: Remove casting during div
> > > > > > >       media: v4l: async: refactor v4l2_async_create_ancillary_links
> > > > > > >       staging: media: tegra-video: Use swap macro
> > > > > > >       media: s2255: Use refcount_t instead of atomic_t for num_channels
> > > > > > >       media: platform: mtk-mdp3: Use refcount_t for job_count
> > > > > > >       media: common: saa7146: Use min macro
> > > > > > >       media: dvb-frontends: drx39xyj: Use min macro
> > > > > > >       media: netup_unidvb: Use min macro
> > > > > > >       media: au0828: Use min macro
> > > > > > >       media: flexcop-usb: Use min macro
> > > > > > >       media: gspca: cpia1: Use min macro
> > > > > > >       media: stk1160: Use min macro
> > > > > > >       media: tegra-vde: Refactor timeout handling
> > > > > > >       media: venus: Use div64_u64
> > > > > > >       media: i2c: st-mipid02: Use the correct div function
> > > > > > >       media: dvb-frontends: tda10048: Use the right div
> > > > > > >       media: tc358746: Use the correct div_ function
> > > > > > >       media: venus: Use the correct div_ function
> > > > > > >       media: venus: Refator return path
> > > > > > >       media: dib0700: Refator return path
> > > > > > >       media: usb: cx231xx: Refator return path
> > > > > > >       media: i2c: rdacm20: Refator return path
> > > > > > >       media: i2c: et8ek8: Refator return path
> > > > > > >       media: cx231xx: Refator return path
> > > > > > >       media: si4713: Refator return path
> > > > > > >       media: ttpci: Refator return path
> > > > > > >       media: hdpvr: Refator return path
> > > > > > >       media: venus: Refator return path
> > > > > > >
> > > > > > >  drivers/media/common/saa7146/saa7146_hlp.c         |  8 +++----
> > > > > > >  drivers/media/dvb-frontends/drx39xyj/drxj.c        |  9 +++-----
> > > > > > >  drivers/media/dvb-frontends/stb0899_drv.c          |  5 -----
> > > > > > >  drivers/media/dvb-frontends/tda10048.c             |  3 +--
> > > > > > >  drivers/media/dvb-frontends/tda18271c2dd.c         |  4 ++--
> > > > > > >  drivers/media/i2c/et8ek8/et8ek8_driver.c           |  4 +++-
> > > > > > >  drivers/media/i2c/rdacm20.c                        |  5 ++++-
> > > > > > >  drivers/media/i2c/st-mipid02.c                     |  2 +-
> > > > > > >  drivers/media/i2c/tc358746.c                       |  3 +--
> > > > > > >  drivers/media/pci/mgb4/mgb4_core.c                 |  4 ++--
> > > > > > >  drivers/media/pci/mgb4/mgb4_regs.c                 |  2 +-
> > > > > > >  drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c  |  2 +-
> > > > > > >  drivers/media/pci/ttpci/budget-core.c              |  5 ++++-
> > > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c   | 10 ++++-----
> > > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.c   |  6 ++---
> > > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-core.h   |  2 +-
> > > > > > >  .../media/platform/mediatek/mdp3/mtk-mdp3-m2m.c    |  6 ++---
> > > > > > >  drivers/media/platform/nvidia/tegra-vde/h264.c     |  6 ++---
> > > > > > >  drivers/media/platform/qcom/venus/vdec.c           | 15 +++++++------
> > > > > > >  drivers/media/platform/qcom/venus/venc.c           | 19 +++++++++-------
> > > > > > >  .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  5 +----
> > > > > > >  drivers/media/radio/si4713/radio-usb-si4713.c      |  8 +++++--
> > > > > > >  drivers/media/usb/au0828/au0828-video.c            |  5 +----
> > > > > > >  drivers/media/usb/b2c2/flexcop-usb.c               |  5 +----
> > > > > > >  drivers/media/usb/cx231xx/cx231xx-i2c.c            | 16 +++++++++----
> > > > > > >  drivers/media/usb/cx231xx/cx231xx-video.c          | 10 +++++++--
> > > > > > >  drivers/media/usb/dvb-usb/dib0700_core.c           |  4 +++-
> > > > > > >  drivers/media/usb/go7007/go7007-fw.c               |  4 ++--
> > > > > > >  drivers/media/usb/gspca/cpia1.c                    |  6 ++---
> > > > > > >  drivers/media/usb/hdpvr/hdpvr-control.c            |  4 +++-
> > > > > > >  drivers/media/usb/s2255/s2255drv.c                 | 20 ++++++++---------
> > > > > > >  drivers/media/usb/stk1160/stk1160-video.c          | 10 ++-------
> > > > > > >  drivers/media/usb/uvc/uvc_ctrl.c                   | 26 ++++++++++++----------
> > > > > > >  drivers/media/v4l2-core/v4l2-async.c               |  8 +++----
> > > > > > >  drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c  |  1 -
> > > > > > >  drivers/staging/media/tegra-video/tegra20.c        |  9 ++------
> > > > > > >  36 files changed, 132 insertions(+), 129 deletions(-)
> > > > > > > ---
> > > > > > > base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> > > > > > > change-id: 20240415-fix-cocci-2df3ef22a6f7
> > > > > > >
> > > > > > > Best regards,
>
> --
> Regards,
>
> Laurent Pinchart
>
Dan Carpenter April 16, 2024, 8:47 a.m. UTC | #12
In my opinion, it's better to just ignore old warnings.

When code is new the warnings are going to be mostly correct.  The
original author is there and knows what the code does.  Someone has
the hardware ready to test any changes.  High value, low burden.

When the code is old only the false positives are left.  No one is
testing the code.  It's low value, high burden.

Plus it puts static checker authors in a difficult place because now
people have to work around our mistakes.  It creates animosity.

Now we have to hold ourselves to a much higher standard for false
positives.  It sounds like I'm complaining and lazy, right?  But Oleg
Drokin has told me previously that I spend too much time trying to
silence false positives instead of working on new code.  He's has a
point which is that actually we have limited amount of time and we have
to make choices about what's the most useful thing we can do.

So what I do and what the zero day bot does is we look at warnings one
time and we re-review old warnings whenever a file is changed.

Kernel developers are very good at addressing static checker warnings
and fixing the real issues...  People sometimes ask me to create a
database of warnings which I have reviewed but the answer is that
anything old can be ignored.  As I write this, I've had a thought that
instead of a database of false positives maybe we should record a
database of real bugs to ensure that the fixes for anything real is
applied.

regards,
dan carpenter
Dan Carpenter April 16, 2024, 10:10 a.m. UTC | #13
On Mon, Apr 15, 2024 at 07:34:40PM +0000, Ricardo Ribalda wrote:
> z does not fit in 32 bits.
> 

z has to fit in 32 bits otherwise there is a different bug.

> Found by cocci:
> drivers/media/dvb-frontends/tda10048.c:345:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/dvb-frontends/tda10048.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/tda10048.c b/drivers/media/dvb-frontends/tda10048.c
> index 5d5e4e9e4422..b176e7803e5b 100644
> --- a/drivers/media/dvb-frontends/tda10048.c
> +++ b/drivers/media/dvb-frontends/tda10048.c
> @@ -342,8 +342,7 @@ static int tda10048_set_wref(struct dvb_frontend *fe, u32 sample_freq_hz,
>  	t *= (2048 * 1024);
>  	t *= 1024;
>  	z = 7 * sample_freq_hz;

sample_freq_hz is a u32 so z can't be more than U32_MAX.  Perhaps there
is an integer overflow bug on this line.

The sample frequency is set in tda10048_set_if().

	state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);

->xtal_hz is set earlier in tda10048_set_if() and it goes up to
16,000,000.  So if ->pll_mfactor is non-zero this line will have an
integer overflow.  16million * 46 > U32_MAX.  Maybe when .clk_freq_khz
is TDA10048_CLK_16000 then ->pll_mfactor is zero?  Ugh...

> -	do_div(t, z);
> -	t += 5;
> +	t = div64_u64(t, z) + 5;
>  	do_div(t, 10);

regards,
dan carpenter
Dan Carpenter April 16, 2024, 10:27 a.m. UTC | #14
I have created a Smatch check to warn about code like this:

drivers/media/dvb-frontends/tda10048.c:345 tda10048_set_wref() warn: unnecessary div64_u64(): divisor = '0-u32max'

regards,
dan carpenter

/*
 * Copyright 2024 Linaro Ltd.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_extra.h"

static int my_id;

static const sval_t uint_max  = { .type = &uint_ctype, .value = UINT_MAX };

static void match_div64_u64(struct expression *expr)
{
	struct range_list *rl;

	get_real_absolute_rl(expr, &rl);
	if (sval_cmp(rl_max(rl), uint_max) > 0)
		return;
	sm_warning("unnecessary div64_u64(): divisor = '%s'", show_rl(rl));
}

void check_unnecessary_div64_u64(int id)
{
	my_id = id;

	if (option_project != PROJ_KERNEL)
		return;

	add_param_key_expr_hook("div64_u64", match_div64_u64, 1, "$", NULL);
}
Ricardo Ribalda April 16, 2024, 10:39 a.m. UTC | #15
Hi Dan

What about going the safe way?

--- a/drivers/media/dvb-frontends/tda10048.c
+++ b/drivers/media/dvb-frontends/tda10048.c
@@ -341,7 +341,7 @@ static int tda10048_set_wref(struct dvb_frontend
*fe, u32 sample_freq_hz,
        /* t *= 2147483648 on 32bit platforms */
        t *= (2048 * 1024);
        t *= 1024;
-       z = 7 * sample_freq_hz;
+       z = (u64)7 * sample_freq_hz;
        t = div64_u64(t, z) + 5;
        do_div(t, 10);

@@ -409,6 +409,7 @@ static int tda10048_set_if(struct dvb_frontend *fe, u32 bw)
        struct tda10048_config *config = &state->config;
        int i;
        u32 if_freq_khz;
+       u64 sample_freq;

        dprintk(1, "%s(bw = %d)\n", __func__, bw);

@@ -450,9 +451,10 @@ static int tda10048_set_if(struct dvb_frontend *fe, u32 bw)
        dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor);

        /* Calculate the sample frequency */
-       state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
-       state->sample_freq /= (state->pll_nfactor + 1);
-       state->sample_freq /= (state->pll_pfactor + 4);
+       sample_freq = (u64)state->xtal_hz * (state->pll_mfactor + 45);
+       do_div(sample_freq, state->pll_nfactor + 1);
+       do_div(sample_freq, state->pll_pfactor + 4);
+       state->sample_freq = sample_freq;
        dprintk(1, "- sample_freq = %d\n", state->sample_freq);

        /* Update the I/F */

I will add a extra patch to fix tda10048_set_if

Thanks

PS: Thanks a lot for your reviews, really appreciate! I have a v2 with
your changes, I am giving it a couple of days before re-submitting

On Tue, 16 Apr 2024 at 12:10, Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Mon, Apr 15, 2024 at 07:34:40PM +0000, Ricardo Ribalda wrote:
> > z does not fit in 32 bits.
> >
>
> z has to fit in 32 bits otherwise there is a different bug.
>
> > Found by cocci:
> > drivers/media/dvb-frontends/tda10048.c:345:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/dvb-frontends/tda10048.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/dvb-frontends/tda10048.c b/drivers/media/dvb-frontends/tda10048.c
> > index 5d5e4e9e4422..b176e7803e5b 100644
> > --- a/drivers/media/dvb-frontends/tda10048.c
> > +++ b/drivers/media/dvb-frontends/tda10048.c
> > @@ -342,8 +342,7 @@ static int tda10048_set_wref(struct dvb_frontend *fe, u32 sample_freq_hz,
> >       t *= (2048 * 1024);
> >       t *= 1024;
> >       z = 7 * sample_freq_hz;
>
> sample_freq_hz is a u32 so z can't be more than U32_MAX.  Perhaps there
> is an integer overflow bug on this line.
>
> The sample frequency is set in tda10048_set_if().
>
>         state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
>
> ->xtal_hz is set earlier in tda10048_set_if() and it goes up to
> 16,000,000.  So if ->pll_mfactor is non-zero this line will have an
> integer overflow.  16million * 46 > U32_MAX.  Maybe when .clk_freq_khz
> is TDA10048_CLK_16000 then ->pll_mfactor is zero?  Ugh...
>
> > -     do_div(t, z);
> > -     t += 5;
> > +     t = div64_u64(t, z) + 5;
> >       do_div(t, 10);
>
> regards,
> dan carpenter
>
Dan Carpenter April 16, 2024, 10:56 a.m. UTC | #16
On Tue, Apr 16, 2024 at 12:39:33PM +0200, Ricardo Ribalda wrote:
> Hi Dan
> 
> What about going the safe way?
> 
> --- a/drivers/media/dvb-frontends/tda10048.c
> +++ b/drivers/media/dvb-frontends/tda10048.c
> @@ -341,7 +341,7 @@ static int tda10048_set_wref(struct dvb_frontend
> *fe, u32 sample_freq_hz,
>         /* t *= 2147483648 on 32bit platforms */
>         t *= (2048 * 1024);
>         t *= 1024;
> -       z = 7 * sample_freq_hz;
> +       z = (u64)7 * sample_freq_hz;

I think your patch works, but it might be nicer without the casts.  We
end up having the discussion where it's like "Can this hz be above 4GHz?"
And, here, I think we're safe but a lot of stuff is pushing up against
that limit these days...

regards,
dan carpenter
Martin Tůma April 16, 2024, 11:30 a.m. UTC | #17
On 15. 04. 24 21:34, Ricardo Ribalda wrote:
> The struct resource end field is inclusive not exclusive, this is, the
> size is (end - start) +1.
> 
> Update the definitions and use the generic resource_size() function.
> 
> Fixes cocci check:
> drivers/media/pci/mgb4/mgb4_regs.c:13:22-25: WARNING: Suspicious code. resource_size is maybe missing with res
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>   drivers/media/pci/mgb4/mgb4_core.c | 4 ++--
>   drivers/media/pci/mgb4/mgb4_regs.c | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/pci/mgb4/mgb4_core.c b/drivers/media/pci/mgb4/mgb4_core.c
> index 9bcf10a77fd3..60498a5abebf 100644
> --- a/drivers/media/pci/mgb4/mgb4_core.c
> +++ b/drivers/media/pci/mgb4/mgb4_core.c
> @@ -493,13 +493,13 @@ static int mgb4_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	struct mgb4_dev *mgbdev;
>   	struct resource video = {
>   		.start	= 0x0,
> -		.end	= 0x100,
> +		.end	= 0xff,
>   		.flags	= IORESOURCE_MEM,
>   		.name	= "mgb4-video",
>   	};
>   	struct resource cmt = {
>   		.start	= 0x1000,
> -		.end	= 0x1800,
> +		.end	= 0x17ff,
>   		.flags	= IORESOURCE_MEM,
>   		.name	= "mgb4-cmt",
>   	};
> diff --git a/drivers/media/pci/mgb4/mgb4_regs.c b/drivers/media/pci/mgb4/mgb4_regs.c
> index 53d4e4503a74..31befd722d72 100644
> --- a/drivers/media/pci/mgb4/mgb4_regs.c
> +++ b/drivers/media/pci/mgb4/mgb4_regs.c
> @@ -10,7 +10,7 @@
>   int mgb4_regs_map(struct resource *res, struct mgb4_regs *regs)
>   {
>   	regs->mapbase = res->start;
> -	regs->mapsize = res->end - res->start;
> +	regs->mapsize = resource_size(res);
>   
>   	if (!request_mem_region(regs->mapbase, regs->mapsize, res->name))
>   		return -EINVAL;
> 

Reviewed-by: Martin Tůma <martin.tuma@digiteqautomotive.com>
Benjamin Mugnier April 17, 2024, 12:55 p.m. UTC | #18
Hi Ricardo,

Thanks a lot.
Reviewed-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

On 4/15/24 21:34, Ricardo Ribalda wrote:
> link_freq does not fit in 32 bits.
> 
> Found by cocci:
> drivers/media/i2c/st-mipid02.c:329:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_s64 instead.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/i2c/st-mipid02.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
> index f250640729ca..93a40bfda1af 100644
> --- a/drivers/media/i2c/st-mipid02.c
> +++ b/drivers/media/i2c/st-mipid02.c
> @@ -326,7 +326,7 @@ static int mipid02_configure_from_rx_speed(struct mipid02_dev *bridge,
>  	}
>  
>  	dev_dbg(&client->dev, "detect link_freq = %lld Hz", link_freq);
> -	do_div(ui_4, link_freq);
> +	ui_4 = div64_s64(ui_4, link_freq);
>  	bridge->r.clk_lane_reg1 |= ui_4 << 2;
>  
>  	return 0;
>
Laurent Pinchart April 17, 2024, 3:51 p.m. UTC | #19
On Tue, Apr 16, 2024 at 11:47:17AM +0300, Dan Carpenter wrote:
> In my opinion, it's better to just ignore old warnings.

I agree. Whatever checkers we enable, whatever code we test, there will
always be false positives. A CI system needs to be able to ignore those
false positives and only warn about new issues.

> When code is new the warnings are going to be mostly correct.  The
> original author is there and knows what the code does.  Someone has
> the hardware ready to test any changes.  High value, low burden.
> 
> When the code is old only the false positives are left.  No one is
> testing the code.  It's low value, high burden.
> 
> Plus it puts static checker authors in a difficult place because now
> people have to work around our mistakes.  It creates animosity.
> 
> Now we have to hold ourselves to a much higher standard for false
> positives.  It sounds like I'm complaining and lazy, right?  But Oleg
> Drokin has told me previously that I spend too much time trying to
> silence false positives instead of working on new code.  He's has a
> point which is that actually we have limited amount of time and we have
> to make choices about what's the most useful thing we can do.
> 
> So what I do and what the zero day bot does is we look at warnings one
> time and we re-review old warnings whenever a file is changed.
> 
> Kernel developers are very good at addressing static checker warnings
> and fixing the real issues...  People sometimes ask me to create a
> database of warnings which I have reviewed but the answer is that
> anything old can be ignored.  As I write this, I've had a thought that
> instead of a database of false positives maybe we should record a
> database of real bugs to ensure that the fixes for anything real is
> applied.
Ricardo Ribalda April 17, 2024, 4:19 p.m. UTC | #20
Hi Laurent

On Wed, 17 Apr 2024 at 17:51, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Apr 16, 2024 at 11:47:17AM +0300, Dan Carpenter wrote:
> > In my opinion, it's better to just ignore old warnings.
>
> I agree. Whatever checkers we enable, whatever code we test, there will
> always be false positives. A CI system needs to be able to ignore those
> false positives and only warn about new issues.

We already have support for that:
https://gitlab.freedesktop.org/linux-media/media-ci/-/tree/main/testdata/static?ref_type=heads

But it would be great if those lists were as small as possible:

- If we have a lot of warnings, two error messages can be combined and
will scape the filters
eg:
print(AAAA);
print(BBBB);
> AABBBAAB

- The filters might hide new errors if they are too broad


Most of the patches in this series are simple and make a nicer code:
Eg the non return minmax() ,
Other patches show real integer overflows.

Now that the patches are ready, let's bite the bullet and try to
reduce our technical debt.


Regards!
>
> > When code is new the warnings are going to be mostly correct.  The
> > original author is there and knows what the code does.  Someone has
> > the hardware ready to test any changes.  High value, low burden.
> >
> > When the code is old only the false positives are left.  No one is
> > testing the code.  It's low value, high burden.
> >
> > Plus it puts static checker authors in a difficult place because now
> > people have to work around our mistakes.  It creates animosity.
> >
> > Now we have to hold ourselves to a much higher standard for false
> > positives.  It sounds like I'm complaining and lazy, right?  But Oleg
> > Drokin has told me previously that I spend too much time trying to
> > silence false positives instead of working on new code.  He's has a
> > point which is that actually we have limited amount of time and we have
> > to make choices about what's the most useful thing we can do.
> >
> > So what I do and what the zero day bot does is we look at warnings one
> > time and we re-review old warnings whenever a file is changed.
> >
> > Kernel developers are very good at addressing static checker warnings
> > and fixing the real issues...  People sometimes ask me to create a
> > database of warnings which I have reviewed but the answer is that
> > anything old can be ignored.  As I write this, I've had a thought that
> > instead of a database of false positives maybe we should record a
> > database of real bugs to ensure that the fixes for anything real is
> > applied.
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart April 18, 2024, 10:53 a.m. UTC | #21
Hi Ricardo,

On Wed, Apr 17, 2024 at 06:19:14PM +0200, Ricardo Ribalda wrote:
> On Wed, 17 Apr 2024 at 17:51, Laurent Pinchart wrote:
> > On Tue, Apr 16, 2024 at 11:47:17AM +0300, Dan Carpenter wrote:
> > > In my opinion, it's better to just ignore old warnings.
> >
> > I agree. Whatever checkers we enable, whatever code we test, there will
> > always be false positives. A CI system needs to be able to ignore those
> > false positives and only warn about new issues.
> 
> We already have support for that:
> https://gitlab.freedesktop.org/linux-media/media-ci/-/tree/main/testdata/static?ref_type=heads

Those are manually written filters. Would it be possible to reduce the
manual step to flagging something as a false positive, and have a
machine build the filters ?

> But it would be great if those lists were as small as possible:
> 
> - If we have a lot of warnings, two error messages can be combined and
> will scape the filters
> eg:
> print(AAAA);
> print(BBBB);
> > AABBBAAB
> 
> - The filters might hide new errors if they are too broad
> 
> 
> Most of the patches in this series are simple and make a nicer code:
> Eg the non return minmax() ,
> Other patches show real integer overflows.
> 
> Now that the patches are ready, let's bite the bullet and try to
> reduce our technical debt.
> 
> > > When code is new the warnings are going to be mostly correct.  The
> > > original author is there and knows what the code does.  Someone has
> > > the hardware ready to test any changes.  High value, low burden.
> > >
> > > When the code is old only the false positives are left.  No one is
> > > testing the code.  It's low value, high burden.
> > >
> > > Plus it puts static checker authors in a difficult place because now
> > > people have to work around our mistakes.  It creates animosity.
> > >
> > > Now we have to hold ourselves to a much higher standard for false
> > > positives.  It sounds like I'm complaining and lazy, right?  But Oleg
> > > Drokin has told me previously that I spend too much time trying to
> > > silence false positives instead of working on new code.  He's has a
> > > point which is that actually we have limited amount of time and we have
> > > to make choices about what's the most useful thing we can do.
> > >
> > > So what I do and what the zero day bot does is we look at warnings one
> > > time and we re-review old warnings whenever a file is changed.
> > >
> > > Kernel developers are very good at addressing static checker warnings
> > > and fixing the real issues...  People sometimes ask me to create a
> > > database of warnings which I have reviewed but the answer is that
> > > anything old can be ignored.  As I write this, I've had a thought that
> > > instead of a database of false positives maybe we should record a
> > > database of real bugs to ensure that the fixes for anything real is
> > > applied.
Laurent Pinchart April 18, 2024, 11:04 a.m. UTC | #22
Hi Ricardo,

Thank you for the patch.

On Mon, Apr 15, 2024 at 07:34:20PM +0000, Ricardo Ribalda wrote:
> Avoid using the iterators after the list_for_each() constructs.
> This patch should be a NOP, but makes cocci, happier:
> 
> drivers/media/usb/uvc/uvc_ctrl.c:1861:44-50: ERROR: invalid reference to the index variable of the iterator on line 1850
> drivers/media/usb/uvc/uvc_ctrl.c:2195:17-23: ERROR: invalid reference to the index variable of the iterator on line 2179
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c2761..a4a987913430 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1850,16 +1850,18 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
>  	list_for_each_entry(entity, &chain->entities, chain) {

If we really want to ensure the iterator won't be used after the loop,
it could be declared in the loop statement itself, now that the kernel
has switched to a newer C version.

>  		ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback,
>  					     &err_ctrl);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			if (ctrls)
> +				ctrls->error_idx =
> +					uvc_ctrl_find_ctrl_idx(entity, ctrls,
> +							       err_ctrl);
>  			goto done;
> +		}
>  	}
>  
>  	if (!rollback)
>  		uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count);
>  done:
> -	if (ret < 0 && ctrls)
> -		ctrls->error_idx = uvc_ctrl_find_ctrl_idx(entity, ctrls,
> -							  err_ctrl);
>  	mutex_unlock(&chain->ctrl_mutex);
>  	return ret;
>  }
> @@ -2165,7 +2167,7 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
>  int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>  	struct uvc_xu_control_query *xqry)
>  {
> -	struct uvc_entity *entity;
> +	struct uvc_entity *entity, *iter;
>  	struct uvc_control *ctrl;
>  	unsigned int i;
>  	bool found;
> @@ -2175,16 +2177,16 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>  	int ret;
>  
>  	/* Find the extension unit. */
> -	found = false;
> -	list_for_each_entry(entity, &chain->entities, chain) {
> -		if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT &&
> -		    entity->id == xqry->unit) {
> -			found = true;
> +	entity = NULL;
> +	list_for_each_entry(iter, &chain->entities, chain) {

Same here, iter could be declared in the loop.

> +		if (UVC_ENTITY_TYPE(iter) == UVC_VC_EXTENSION_UNIT &&
> +		    iter->id == xqry->unit) {
> +			entity = iter;
>  			break;
>  		}
>  	}
>  
> -	if (!found) {
> +	if (!entity) {
>  		uvc_dbg(chain->dev, CONTROL, "Extension unit %u not found\n",
>  			xqry->unit);
>  		return -ENOENT;
>
Ricardo Ribalda April 18, 2024, 2:51 p.m. UTC | #23
Hi Laurent

On Thu, 18 Apr 2024 at 12:53, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> On Wed, Apr 17, 2024 at 06:19:14PM +0200, Ricardo Ribalda wrote:
> > On Wed, 17 Apr 2024 at 17:51, Laurent Pinchart wrote:
> > > On Tue, Apr 16, 2024 at 11:47:17AM +0300, Dan Carpenter wrote:
> > > > In my opinion, it's better to just ignore old warnings.
> > >
> > > I agree. Whatever checkers we enable, whatever code we test, there will
> > > always be false positives. A CI system needs to be able to ignore those
> > > false positives and only warn about new issues.
> >
> > We already have support for that:
> > https://gitlab.freedesktop.org/linux-media/media-ci/-/tree/main/testdata/static?ref_type=heads
>
> Those are manually written filters. Would it be possible to reduce the
> manual step to flagging something as a false positive, and have a
> machine build the filters ?
>

Do you expect that the list of exceptions will grow?

I hope that once the CI is in place we will fix the warnings before
they land in the tree.


> > But it would be great if those lists were as small as possible:
> >
> > - If we have a lot of warnings, two error messages can be combined and
> > will scape the filters
> > eg:
> > print(AAAA);
> > print(BBBB);
> > > AABBBAAB
> >
> > - The filters might hide new errors if they are too broad
> >
> >
> > Most of the patches in this series are simple and make a nicer code:
> > Eg the non return minmax() ,
> > Other patches show real integer overflows.
> >
> > Now that the patches are ready, let's bite the bullet and try to
> > reduce our technical debt.
> >
> > > > When code is new the warnings are going to be mostly correct.  The
> > > > original author is there and knows what the code does.  Someone has
> > > > the hardware ready to test any changes.  High value, low burden.
> > > >
> > > > When the code is old only the false positives are left.  No one is
> > > > testing the code.  It's low value, high burden.
> > > >
> > > > Plus it puts static checker authors in a difficult place because now
> > > > people have to work around our mistakes.  It creates animosity.
> > > >
> > > > Now we have to hold ourselves to a much higher standard for false
> > > > positives.  It sounds like I'm complaining and lazy, right?  But Oleg
> > > > Drokin has told me previously that I spend too much time trying to
> > > > silence false positives instead of working on new code.  He's has a
> > > > point which is that actually we have limited amount of time and we have
> > > > to make choices about what's the most useful thing we can do.
> > > >
> > > > So what I do and what the zero day bot does is we look at warnings one
> > > > time and we re-review old warnings whenever a file is changed.
> > > >
> > > > Kernel developers are very good at addressing static checker warnings
> > > > and fixing the real issues...  People sometimes ask me to create a
> > > > database of warnings which I have reviewed but the answer is that
> > > > anything old can be ignored.  As I write this, I've had a thought that
> > > > instead of a database of false positives maybe we should record a
> > > > database of real bugs to ensure that the fixes for anything real is
> > > > applied.
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart April 18, 2024, 3:46 p.m. UTC | #24
Hi Ricardo,

On Thu, Apr 18, 2024 at 04:51:06PM +0200, Ricardo Ribalda wrote:
> On Thu, 18 Apr 2024 at 12:53, Laurent Pinchart wrote:
> > On Wed, Apr 17, 2024 at 06:19:14PM +0200, Ricardo Ribalda wrote:
> > > On Wed, 17 Apr 2024 at 17:51, Laurent Pinchart wrote:
> > > > On Tue, Apr 16, 2024 at 11:47:17AM +0300, Dan Carpenter wrote:
> > > > > In my opinion, it's better to just ignore old warnings.
> > > >
> > > > I agree. Whatever checkers we enable, whatever code we test, there will
> > > > always be false positives. A CI system needs to be able to ignore those
> > > > false positives and only warn about new issues.
> > >
> > > We already have support for that:
> > > https://gitlab.freedesktop.org/linux-media/media-ci/-/tree/main/testdata/static?ref_type=heads
> >
> > Those are manually written filters. Would it be possible to reduce the
> > manual step to flagging something as a false positive, and have a
> > machine build the filters ?
> 
> Do you expect that the list of exceptions will grow?
> 
> I hope that once the CI is in place we will fix the warnings before
> they land in the tree.

Any static checker is bound to produce false positives. Some of them can
be addressed by improving the checker, others by modifying the source
code, but in some cases the first option would be too difficult and the
second would reduce readability of the code. I thus thing the list of
accepted false positives will grow over time.

> > > But it would be great if those lists were as small as possible:
> > >
> > > - If we have a lot of warnings, two error messages can be combined and
> > > will scape the filters
> > > eg:
> > > print(AAAA);
> > > print(BBBB);
> > > > AABBBAAB
> > >
> > > - The filters might hide new errors if they are too broad
> > >
> > >
> > > Most of the patches in this series are simple and make a nicer code:
> > > Eg the non return minmax() ,
> > > Other patches show real integer overflows.
> > >
> > > Now that the patches are ready, let's bite the bullet and try to
> > > reduce our technical debt.
> > >
> > > > > When code is new the warnings are going to be mostly correct.  The
> > > > > original author is there and knows what the code does.  Someone has
> > > > > the hardware ready to test any changes.  High value, low burden.
> > > > >
> > > > > When the code is old only the false positives are left.  No one is
> > > > > testing the code.  It's low value, high burden.
> > > > >
> > > > > Plus it puts static checker authors in a difficult place because now
> > > > > people have to work around our mistakes.  It creates animosity.
> > > > >
> > > > > Now we have to hold ourselves to a much higher standard for false
> > > > > positives.  It sounds like I'm complaining and lazy, right?  But Oleg
> > > > > Drokin has told me previously that I spend too much time trying to
> > > > > silence false positives instead of working on new code.  He's has a
> > > > > point which is that actually we have limited amount of time and we have
> > > > > to make choices about what's the most useful thing we can do.
> > > > >
> > > > > So what I do and what the zero day bot does is we look at warnings one
> > > > > time and we re-review old warnings whenever a file is changed.
> > > > >
> > > > > Kernel developers are very good at addressing static checker warnings
> > > > > and fixing the real issues...  People sometimes ask me to create a
> > > > > database of warnings which I have reviewed but the answer is that
> > > > > anything old can be ignored.  As I write this, I've had a thought that
> > > > > instead of a database of false positives maybe we should record a
> > > > > database of real bugs to ensure that the fixes for anything real is
> > > > > applied.
Ricardo Ribalda April 29, 2024, 11:39 a.m. UTC | #25
Hi Dan

On Tue, 23 Apr 2024 at 11:55, Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Btw, here is the output from my check (based on linux next from a few
> days ago).  There are some false positives because Smatch parses
> __pow10() incorrectly etc but it's mostly correct.

This looks pretty cool :)

Are you planning to add this to smatch soon?

Thanks!