| Message ID | 20240415-fix-cocci-v1-0-477afb23728b@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: Fix coccinelle warning/errors | expand |
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
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
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,
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 >
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
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.
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
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.
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
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.
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,