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 <[email protected]>
---
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 <[email protected]>
chip_id is an unsigned number, it can never be < 0
Fixes cocci check:
drivers/media/dvb-frontends/stb0899_drv.c:1280:8-15: WARNING: Unsigned expression compared with zero: chip_id > 0
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/dvb-frontends/stb0899_drv.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c
index 2f4d8fb400cd..222b5476ebfd 100644
--- a/drivers/media/dvb-frontends/stb0899_drv.c
+++ b/drivers/media/dvb-frontends/stb0899_drv.c
@@ -1277,11 +1277,6 @@ static int stb0899_get_dev_id(struct stb0899_state *state)
dprintk(state->verbose, FE_ERROR, 1, "Demodulator Core ID=[%s], Version=[%d]", (char *) &demod_str, demod_ver);
CONVERT32(STB0899_READ_S2REG(STB0899_S2FEC, FEC_CORE_ID_REG), (char *)&fec_str);
fec_ver = STB0899_READ_S2REG(STB0899_S2FEC, FEC_VER_ID_REG);
- if (! (chip_id > 0)) {
- dprintk(state->verbose, FE_ERROR, 1, "couldn't find a STB 0899");
-
- return -ENODEV;
- }
dprintk(state->verbose, FE_ERROR, 1, "FEC Core ID=[%s], Version=[%d]", (char*) &fec_str, fec_ver);
return 0;
--
2.44.0.683.g7961c838ac-goog
It makes the code simpler and cocci happier:
drivers/media/usb/go7007/go7007-fw.c:1292:14-15: WARNING opportunity for max()
drivers/media/usb/go7007/go7007-fw.c:1293:14-15: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/go7007/go7007-fw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/go7007/go7007-fw.c b/drivers/media/usb/go7007/go7007-fw.c
index 018019ba47d4..86ce593e0c54 100644
--- a/drivers/media/usb/go7007/go7007-fw.c
+++ b/drivers/media/usb/go7007/go7007-fw.c
@@ -1289,8 +1289,8 @@ static int avsync_to_package(struct go7007 *go, __le16 *code, int space)
0xbf99, (u16)((-adjratio) >> 16),
0xbf92, 0,
0xbf93, 0,
- 0xbff4, f1 > f2 ? f1 : f2,
- 0xbff5, f1 < f2 ? f1 : f2,
+ 0xbff4, max(f1, f2),
+ 0xbff5, min(f1, f2),
0xbff6, f1 < f2 ? ratio : ratio + 1,
0xbff7, f1 > f2 ? ratio : ratio + 1,
0xbff8, 0,
--
2.44.0.683.g7961c838ac-goog
It makes the code slightly more clear and makes cocci incredibly happy:
drivers/media/usb/uvc/uvc_ctrl.c:839:22-23: WARNING opportunity for max()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index a4a987913430..4b685f883e4d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -836,7 +836,7 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
while (1) {
u8 byte = *data & mask;
value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
- bits -= 8 - (offset > 0 ? offset : 0);
+ bits -= 8 - max(offset, 0);
if (bits <= 0)
break;
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
---
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) {
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) {
+ 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;
--
2.44.0.683.g7961c838ac-goog
platform_get_irq() already prints an error message.
Found by cocci:
drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c:444:3-10: line 444 is redundant because platform_get_irq() already prints an error
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
index bce821eb71ce..c25027b0ca32 100644
--- a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
+++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
@@ -439,11 +439,8 @@ static int dcmipp_probe(struct platform_device *pdev)
"Could not get reset control\n");
irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
- if (irq != -EPROBE_DEFER)
- dev_err(&pdev->dev, "Could not get irq\n");
+ if (irq <= 0)
return irq ? irq : -ENXIO;
- }
dcmipp->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
if (IS_ERR(dcmipp->regs)) {
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
---
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;
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
}
static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
--
2.44.0.683.g7961c838ac-goog
Makes the code simpler and cocci happier:
drivers/staging/media/tegra-video/tegra20.c:324:44-45: WARNING opportunity for swap()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/staging/media/tegra-video/tegra20.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c
index 630e2ff987a3..7b8f8f810b35 100644
--- a/drivers/staging/media/tegra-video/tegra20.c
+++ b/drivers/staging/media/tegra-video/tegra20.c
@@ -317,13 +317,8 @@ static void tegra20_channel_queue_setup(struct tegra_vi_channel *chan)
chan->addr_offset_v = chan->addr_offset_u + stride * height / 4;
/* For YVU420, we swap the locations of the U and V planes. */
- if (chan->format.pixelformat == V4L2_PIX_FMT_YVU420) {
- unsigned long temp;
-
- temp = chan->addr_offset_u;
- chan->addr_offset_u = chan->addr_offset_v;
- chan->addr_offset_v = temp;
- }
+ if (chan->format.pixelformat == V4L2_PIX_FMT_YVU420)
+ swap(chan->addr_offset_u, chan->addr_offset_v);
chan->start_offset_u = chan->addr_offset_u;
chan->start_offset_v = chan->addr_offset_v;
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
---
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;
goto error_clock_rate_exclusive;
}
--
2.44.0.683.g7961c838ac-goog
do_div() divides 64 bits by 32. We were adding a casting to the divider
to 64 bits, for a number that fits perfectly in 32 bits. Remove it.
Found by cocci:
drivers/media/dvb-frontends/tda18271c2dd.c:355:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
drivers/media/dvb-frontends/tda18271c2dd.c:331:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/dvb-frontends/tda18271c2dd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/tda18271c2dd.c b/drivers/media/dvb-frontends/tda18271c2dd.c
index a34834487943..fd928787207e 100644
--- a/drivers/media/dvb-frontends/tda18271c2dd.c
+++ b/drivers/media/dvb-frontends/tda18271c2dd.c
@@ -328,7 +328,7 @@ static int CalcMainPLL(struct tda_state *state, u32 freq)
OscFreq = (u64) freq * (u64) Div;
OscFreq *= (u64) 16384;
- do_div(OscFreq, (u64)16000000);
+ do_div(OscFreq, 16000000);
MainDiv = OscFreq;
state->m_Regs[MPD] = PostDiv & 0x77;
@@ -352,7 +352,7 @@ static int CalcCalPLL(struct tda_state *state, u32 freq)
OscFreq = (u64)freq * (u64)Div;
/* CalDiv = u32( OscFreq * 16384 / 16000000 ); */
OscFreq *= (u64)16384;
- do_div(OscFreq, (u64)16000000);
+ do_div(OscFreq, 16000000);
CalDiv = OscFreq;
state->m_Regs[CPD] = PostDiv;
--
2.44.0.683.g7961c838ac-goog
Use an API that resembles more the actual use of job_count.
Found by cocci:
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:527:5-24: WARNING: atomic_dec_and_test variation before object free at line 541.
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:578:6-25: WARNING: atomic_dec_and_test variation before object free at line 581.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 10 +++++-----
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c | 6 +++---
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h | 2 +-
drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
index 1d64bac34b90..ea2ea119dd2a 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
@@ -524,7 +524,7 @@ static void mdp_auto_release_work(struct work_struct *work)
mdp_comp_clocks_off(&mdp->pdev->dev, cmd->comps,
cmd->num_comps);
- if (atomic_dec_and_test(&mdp->job_count)) {
+ if (refcount_dec_and_test(&mdp->job_count)) {
if (cmd->mdp_ctx)
mdp_m2m_job_finish(cmd->mdp_ctx);
@@ -575,7 +575,7 @@ static void mdp_handle_cmdq_callback(struct mbox_client *cl, void *mssg)
mdp_comp_clocks_off(&mdp->pdev->dev, cmd->comps,
cmd->num_comps);
- if (atomic_dec_and_test(&mdp->job_count))
+ if (refcount_dec_and_test(&mdp->job_count))
wake_up(&mdp->callback_wq);
mdp_cmdq_pkt_destroy(&cmd->pkt);
@@ -724,9 +724,9 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
int i, ret;
u8 pp_used = __get_pp_num(param->param->type);
- atomic_set(&mdp->job_count, pp_used);
+ refcount_set(&mdp->job_count, pp_used);
if (atomic_read(&mdp->suspended)) {
- atomic_set(&mdp->job_count, 0);
+ refcount_set(&mdp->job_count, 0);
return -ECANCELED;
}
@@ -764,7 +764,7 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
mdp_comp_clocks_off(&mdp->pdev->dev, cmd[i]->comps,
cmd[i]->num_comps);
err_cancel_job:
- atomic_set(&mdp->job_count, 0);
+ refcount_set(&mdp->job_count, 0);
return ret;
}
diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c
index 5209f531ef8d..c1f3bf98120a 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c
@@ -380,14 +380,14 @@ static int __maybe_unused mdp_suspend(struct device *dev)
atomic_set(&mdp->suspended, 1);
- if (atomic_read(&mdp->job_count)) {
+ if (refcount_read(&mdp->job_count)) {
ret = wait_event_timeout(mdp->callback_wq,
- !atomic_read(&mdp->job_count),
+ !refcount_read(&mdp->job_count),
2 * HZ);
if (ret == 0) {
dev_err(dev,
"%s:flushed cmdq task incomplete, count=%d\n",
- __func__, atomic_read(&mdp->job_count));
+ __func__, refcount_read(&mdp->job_count));
return -EBUSY;
}
}
diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h
index 8c09e984fd01..430251f63754 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h
@@ -134,7 +134,7 @@ struct mdp_dev {
/* synchronization protect for m2m device operation */
struct mutex m2m_lock;
atomic_t suspended;
- atomic_t job_count;
+ refcount_t job_count;
};
struct mdp_pipe_info {
diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
index 35a8b059bde5..0e69128a3772 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
@@ -104,14 +104,14 @@ static void mdp_m2m_device_run(void *priv)
task.cb_data = NULL;
task.mdp_ctx = ctx;
- if (atomic_read(&ctx->mdp_dev->job_count)) {
+ if (refcount_read(&ctx->mdp_dev->job_count)) {
ret = wait_event_timeout(ctx->mdp_dev->callback_wq,
- !atomic_read(&ctx->mdp_dev->job_count),
+ !refcount_read(&ctx->mdp_dev->job_count),
2 * HZ);
if (ret == 0) {
dev_err(&ctx->mdp_dev->pdev->dev,
"%d jobs not yet done\n",
- atomic_read(&ctx->mdp_dev->job_count));
+ refcount_read(&ctx->mdp_dev->job_count));
goto worker_end;
}
}
--
2.44.0.683.g7961c838ac-goog
Simplifies the code.
Found by cocci:
drivers/media/dvb-frontends/drx39xyj/drxj.c:1447:23-24: WARNING opportunity for min()
drivers/media/dvb-frontends/drx39xyj/drxj.c:1662:21-22: WARNING opportunity for min()
drivers/media/dvb-frontends/drx39xyj/drxj.c:1685:24-25: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/dvb-frontends/drx39xyj/drxj.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c
index 19d8de400a68..d41ce75575bb 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drxj.c
+++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c
@@ -1444,8 +1444,7 @@ static int drxdap_fasi_read_block(struct i2c_device_addr *dev_addr,
/* Read block from I2C **************************************************** */
do {
- u16 todo = (datasize < DRXDAP_MAX_RCHUNKSIZE ?
- datasize : DRXDAP_MAX_RCHUNKSIZE);
+ u16 todo = min(datasize, DRXDAP_MAX_RCHUNKSIZE);
bufx = 0;
@@ -1659,7 +1658,7 @@ static int drxdap_fasi_write_block(struct i2c_device_addr *dev_addr,
Address must be rewritten because HI is reset after data transport and
expects an address.
*/
- todo = (block_size < datasize ? block_size : datasize);
+ todo = min(block_size, datasize);
if (todo == 0) {
u16 overhead_size_i2c_addr = 0;
u16 data_block_size = 0;
@@ -1681,9 +1680,7 @@ static int drxdap_fasi_write_block(struct i2c_device_addr *dev_addr,
first_err = st;
}
bufx = 0;
- todo =
- (data_block_size <
- datasize ? data_block_size : datasize);
+ todo = min(data_block_size, datasize);
}
memcpy(&buf[bufx], data, todo);
/* write (address if can do and) data */
--
2.44.0.683.g7961c838ac-goog
Simplify the code.
Found by cocci:
drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c:138:26-27: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c b/drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c
index 46676f2c89c7..1c885d620b75 100644
--- a/drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c
+++ b/drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c
@@ -135,7 +135,7 @@ static void netup_i2c_fifo_tx(struct netup_i2c *i2c)
(readw(&i2c->regs->tx_fifo.stat_ctrl) & 0x3f);
u32 msg_length = i2c->msg->len - i2c->xmit_size;
- msg_length = (msg_length < fifo_space ? msg_length : fifo_space);
+ msg_length = min(msg_length, fifo_space);
while (msg_length--) {
data = i2c->msg->buf[i2c->xmit_size++];
writeb(data, &i2c->regs->tx_fifo.data8);
--
2.44.0.683.g7961c838ac-goog
Simplifies the code.
Found by cocci:
drivers/media/usb/b2c2/flexcop-usb.c:201:8-9: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/b2c2/flexcop-usb.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/media/usb/b2c2/flexcop-usb.c b/drivers/media/usb/b2c2/flexcop-usb.c
index 790787f0eba8..3ba6a5ed7167 100644
--- a/drivers/media/usb/b2c2/flexcop-usb.c
+++ b/drivers/media/usb/b2c2/flexcop-usb.c
@@ -197,10 +197,7 @@ static int flexcop_usb_memory_req(struct flexcop_usb *fc_usb,
return -EINVAL;
}
for (i = 0; i < len;) {
- pagechunk =
- wMax < bytes_left_to_read_on_page(addr, len) ?
- wMax :
- bytes_left_to_read_on_page(addr, len);
+ pagechunk = min(wMax, bytes_left_to_read_on_page(addr, len));
deb_info("%x\n",
(addr & V8_MEMORY_PAGE_MASK) |
(V8_MEMORY_EXTENDED*extended));
--
2.44.0.683.g7961c838ac-goog
Use an API that resembles more the actual use of num_channels.
Found by cocci:
drivers/media/usb/s2255/s2255drv.c:2362:5-24: WARNING: atomic_dec_and_test variation before object free at line 2363.
drivers/media/usb/s2255/s2255drv.c:1557:5-24: WARNING: atomic_dec_and_test variation before object free at line 1558.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/s2255/s2255drv.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 8e1de1e8bd12..a6e450181fd0 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -247,7 +247,7 @@ struct s2255_vc {
struct s2255_dev {
struct s2255_vc vc[MAX_CHANNELS];
struct v4l2_device v4l2_dev;
- atomic_t num_channels;
+ refcount_t num_channels;
int frames;
struct mutex lock; /* channels[].vdev.lock */
struct mutex cmdlock; /* protects cmdbuf */
@@ -1550,11 +1550,11 @@ static void s2255_video_device_release(struct video_device *vdev)
container_of(vdev, struct s2255_vc, vdev);
dprintk(dev, 4, "%s, chnls: %d\n", __func__,
- atomic_read(&dev->num_channels));
+ refcount_read(&dev->num_channels));
v4l2_ctrl_handler_free(&vc->hdl);
- if (atomic_dec_and_test(&dev->num_channels))
+ if (refcount_dec_and_test(&dev->num_channels))
s2255_destroy(dev);
return;
}
@@ -1659,7 +1659,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
"failed to register video device!\n");
break;
}
- atomic_inc(&dev->num_channels);
+ refcount_inc(&dev->num_channels);
v4l2_info(&dev->v4l2_dev, "V4L2 device registered as %s\n",
video_device_node_name(&vc->vdev));
@@ -1667,11 +1667,11 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
pr_info("Sensoray 2255 V4L driver Revision: %s\n",
S2255_VERSION);
/* if no channels registered, return error and probe will fail*/
- if (atomic_read(&dev->num_channels) == 0) {
+ if (refcount_read(&dev->num_channels) == 0) {
v4l2_device_unregister(&dev->v4l2_dev);
return ret;
}
- if (atomic_read(&dev->num_channels) != MAX_CHANNELS)
+ if (refcount_read(&dev->num_channels) != MAX_CHANNELS)
pr_warn("s2255: Not all channels available.\n");
return 0;
}
@@ -2221,7 +2221,7 @@ static int s2255_probe(struct usb_interface *interface,
goto errorFWDATA1;
}
- atomic_set(&dev->num_channels, 0);
+ refcount_set(&dev->num_channels, 0);
dev->pid = id->idProduct;
dev->fw_data = kzalloc(sizeof(struct s2255_fw), GFP_KERNEL);
if (!dev->fw_data)
@@ -2341,12 +2341,12 @@ static void s2255_disconnect(struct usb_interface *interface)
{
struct s2255_dev *dev = to_s2255_dev(usb_get_intfdata(interface));
int i;
- int channels = atomic_read(&dev->num_channels);
+ int channels = refcount_read(&dev->num_channels);
mutex_lock(&dev->lock);
v4l2_device_disconnect(&dev->v4l2_dev);
mutex_unlock(&dev->lock);
/*see comments in the uvc_driver.c usb disconnect function */
- atomic_inc(&dev->num_channels);
+ refcount_inc(&dev->num_channels);
/* unregister each video device. */
for (i = 0; i < channels; i++)
video_unregister_device(&dev->vc[i].vdev);
@@ -2359,7 +2359,7 @@ static void s2255_disconnect(struct usb_interface *interface)
dev->vc[i].vidstatus_ready = 1;
wake_up(&dev->vc[i].wait_vidstatus);
}
- if (atomic_dec_and_test(&dev->num_channels))
+ if (refcount_dec_and_test(&dev->num_channels))
s2255_destroy(dev);
dev_info(&interface->dev, "%s\n", __func__);
}
--
2.44.0.683.g7961c838ac-goog
Simplifies the code.
Found by cocci:
drivers/media/usb/gspca/cpia1.c:607:30-31: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/gspca/cpia1.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/gspca/cpia1.c b/drivers/media/usb/gspca/cpia1.c
index 5f5fa851ca64..14aaf36cde6e 100644
--- a/drivers/media/usb/gspca/cpia1.c
+++ b/drivers/media/usb/gspca/cpia1.c
@@ -604,10 +604,8 @@ static int find_over_exposure(int brightness)
MaxAllowableOverExposure = FLICKER_MAX_EXPOSURE - brightness -
FLICKER_BRIGHTNESS_CONSTANT;
- if (MaxAllowableOverExposure < FLICKER_ALLOWABLE_OVER_EXPOSURE)
- OverExposure = MaxAllowableOverExposure;
- else
- OverExposure = FLICKER_ALLOWABLE_OVER_EXPOSURE;
+ OverExposure = min(MaxAllowableOverExposure,
+ FLICKER_ALLOWABLE_OVER_EXPOSURE);
return OverExposure;
}
--
2.44.0.683.g7961c838ac-goog
Simplifies the code.
Found by cocci:
drivers/media/usb/au0828/au0828-video.c:605:11-12: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/au0828/au0828-video.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index fd9fc43d47e0..2ec49ea479d5 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -602,10 +602,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
vbi_field_size = dev->vbi_width * dev->vbi_height * 2;
if (dev->vbi_read < vbi_field_size) {
remain = vbi_field_size - dev->vbi_read;
- if (len < remain)
- lencopy = len;
- else
- lencopy = remain;
+ lencopy = umin(len, remain);
if (vbi_buf != NULL)
au0828_copy_vbi(dev, vbi_dma_q, vbi_buf, p,
--
2.44.0.683.g7961c838ac-goog
Simplifies the code. Found by cocci:
drivers/media/common/saa7146/saa7146_hlp.c:125:36-37: WARNING opportunity for min()
drivers/media/common/saa7146/saa7146_hlp.c:154:41-42: WARNING opportunity for min()
drivers/media/common/saa7146/saa7146_hlp.c:286:35-36: WARNING opportunity for min()
drivers/media/common/saa7146/saa7146_hlp.c:289:35-36: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/common/saa7146/saa7146_hlp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_hlp.c b/drivers/media/common/saa7146/saa7146_hlp.c
index 7569d8cdd4d8..fe3348af543e 100644
--- a/drivers/media/common/saa7146/saa7146_hlp.c
+++ b/drivers/media/common/saa7146/saa7146_hlp.c
@@ -122,7 +122,7 @@ static int calculate_h_scale_registers(struct saa7146_dev *dev,
xacm = 0;
/* set horizontal filter parameters (CXY = CXUV) */
- cxy = hps_h_coeff_tab[( (xpsc - 1) < 63 ? (xpsc - 1) : 63 )].hps_coeff;
+ cxy = hps_h_coeff_tab[min(xpsc - 1, 63)].hps_coeff;
cxuv = cxy;
/* calculate and set horizontal fine scale (xsci) */
@@ -151,7 +151,7 @@ static int calculate_h_scale_registers(struct saa7146_dev *dev,
xacm = 0;
/* get best match in the table of attenuations
for horizontal scaling */
- h_atten = hps_h_coeff_tab[( (xpsc - 1) < 63 ? (xpsc - 1) : 63 )].weight_sum;
+ h_atten = hps_h_coeff_tab[min(xpsc - 1, 63)].weight_sum;
for (i = 0; h_attenuation[i] != 0; i++) {
if (h_attenuation[i] >= h_atten)
@@ -283,10 +283,10 @@ static int calculate_v_scale_registers(struct saa7146_dev *dev, enum v4l2_field
}
/* get filter coefficients for cya, cyb from table hps_v_coeff_tab */
- cya_cyb = hps_v_coeff_tab[ (yacl < 63 ? yacl : 63 ) ].hps_coeff;
+ cya_cyb = hps_v_coeff_tab[min(yacl, 63)].hps_coeff;
/* get best match in the table of attenuations for vertical scaling */
- v_atten = hps_v_coeff_tab[ (yacl < 63 ? yacl : 63 ) ].weight_sum;
+ v_atten = hps_v_coeff_tab[min(yacl, 63)].weight_sum;
for (i = 0; v_attenuation[i] != 0; i++) {
if (v_attenuation[i] >= v_atten)
--
2.44.0.683.g7961c838ac-goog
Simplifies the code.
Found by cocci:
drivers/media/usb/stk1160/stk1160-video.c:133:12-13: WARNING opportunity for min()
drivers/media/usb/stk1160/stk1160-video.c:176:13-14: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/stk1160/stk1160-video.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/stk1160/stk1160-video.c b/drivers/media/usb/stk1160/stk1160-video.c
index 366f0e4a5dc0..0ba0f41fe3f4 100644
--- a/drivers/media/usb/stk1160/stk1160-video.c
+++ b/drivers/media/usb/stk1160/stk1160-video.c
@@ -130,10 +130,7 @@ void stk1160_copy_video(struct stk1160 *dev, u8 *src, int len)
dst += linesdone * bytesperline * 2 + lineoff;
/* Copy the remaining of current line */
- if (remain < (bytesperline - lineoff))
- lencopy = remain;
- else
- lencopy = bytesperline - lineoff;
+ lencopy = min(remain, bytesperline - lineoff);
/*
* Check if we have enough space left in the buffer.
@@ -173,10 +170,7 @@ void stk1160_copy_video(struct stk1160 *dev, u8 *src, int len)
src += lencopy;
/* Copy one line at a time */
- if (remain < bytesperline)
- lencopy = remain;
- else
- lencopy = bytesperline;
+ lencopy = min(remain, bytesperline);
/*
* Check if we have enough space left in the buffer.
--
2.44.0.683.g7961c838ac-goog
us_per_frame does not fit in 32 bits. Use the appropriate div function.
Found by cocci:
drivers/media/platform/qcom/venus/vdec.c:488:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 29130a9441e7..1dffeb7de76e 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -464,7 +464,7 @@ static int vdec_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
struct venus_inst *inst = to_inst(file);
struct v4l2_captureparm *cap = &a->parm.capture;
struct v4l2_fract *timeperframe = &cap->timeperframe;
- u64 us_per_frame, fps;
+ u64 us_per_frame;
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
@@ -484,10 +484,7 @@ static int vdec_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
if (!us_per_frame)
return -EINVAL;
- fps = (u64)USEC_PER_SEC;
- do_div(fps, us_per_frame);
-
- inst->fps = fps;
+ inst->fps = div64_u64(USEC_PER_SEC, us_per_frame);
inst->timeperframe = *timeperframe;
return 0;
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
---
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;
--
2.44.0.683.g7961c838ac-goog
fin does not fit in 32 bits in some arches.
Found by cocci:
drivers/media/i2c/tc358746.c:847:2-8: WARNING: do_div() does a 64-by-32 division, please consider using div64_ul instead.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/i2c/tc358746.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c
index d676adc4401b..edf79107adc5 100644
--- a/drivers/media/i2c/tc358746.c
+++ b/drivers/media/i2c/tc358746.c
@@ -844,8 +844,7 @@ static unsigned long tc358746_find_pll_settings(struct tc358746 *tc358746,
continue;
tmp = fout * postdiv;
- do_div(tmp, fin);
- mul = tmp;
+ mul = div64_ul(tmp, fin);
if (mul > 511)
continue;
--
2.44.0.683.g7961c838ac-goog
us_per_frame does not fit in u32
Found with cocci:
drivers/media/platform/qcom/venus/venc.c:418:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/qcom/venus/venc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 3ec2fb8d9fab..de06e1712d09 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -393,7 +393,7 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
struct venus_inst *inst = to_inst(file);
struct v4l2_outputparm *out = &a->parm.output;
struct v4l2_fract *timeperframe = &out->timeperframe;
- u64 us_per_frame, fps;
+ u64 us_per_frame;
if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
@@ -414,11 +414,8 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
if (!us_per_frame)
return -EINVAL;
- fps = (u64)USEC_PER_SEC;
- do_div(fps, us_per_frame);
-
+ inst->fps = div64_u64(USEC_PER_SEC, us_per_frame);
inst->timeperframe = *timeperframe;
- inst->fps = fps;
return 0;
}
--
2.44.0.683.g7961c838ac-goog
z does not fit in 32 bits.
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 <[email protected]>
---
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;
- do_div(t, z);
- t += 5;
+ t = div64_u64(t, z) + 5;
do_div(t, 10);
tda10048_writereg(state, TDA10048_TIME_WREF_LSB, (u8)t);
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/platform/qcom/venus/vdec.c:672:12-13: WARNING opportunity for min()
drivers/media/platform/qcom/venus/vdec.c:650:12-13: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 1dffeb7de76e..2bc7aecb35e5 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -647,7 +647,9 @@ static int vdec_pm_put(struct venus_inst *inst, bool autosuspend)
mutex_unlock(&core->pm_lock);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
static int vdec_pm_get_put(struct venus_inst *inst)
@@ -669,7 +671,9 @@ static int vdec_pm_get_put(struct venus_inst *inst)
error:
mutex_unlock(&core->pm_lock);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
static void vdec_pm_touch(struct venus_inst *inst)
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/usb/dvb-usb/dib0700_core.c:67:15-16: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/dvb-usb/dib0700_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index 1caabb51ea47..b2ad2d0f76f8 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -64,7 +64,9 @@ static int dib0700_ctrl_wr(struct dvb_usb_device *d, u8 *tx, u8 txlen)
if (status != txlen)
deb_data("ep 0 write error (status = %d, len: %d)\n",status,txlen);
- return status < 0 ? status : 0;
+ if (status < 0)
+ return status;
+ return 0;
}
/* expecting tx buffer: request data[0] ... data[n] (n <= 4) */
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
rivers/media/usb/cx231xx/cx231xx-video.c:1282:12-13: WARNING opportunity for min()
drivers/media/usb/cx231xx/cx231xx-video.c:1328:12-13: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/cx231xx/cx231xx-video.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 8f347bbeeb32..bbe207d72427 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1279,7 +1279,10 @@ int cx231xx_g_register(struct file *file, void *priv,
default:
return -EINVAL;
}
- return ret < 0 ? ret : 0;
+
+ if (ret < 0)
+ return ret;
+ return 0;
}
int cx231xx_s_register(struct file *file, void *priv,
@@ -1325,7 +1328,10 @@ int cx231xx_s_register(struct file *file, void *priv,
default:
return -EINVAL;
}
- return ret < 0 ? ret : 0;
+
+ if (ret < 0)
+ return ret;
+ return 0;
}
#endif
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/i2c/rdacm20.c:363:12-13: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/i2c/rdacm20.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
index b4647bda8c21..b40d40b0cdd4 100644
--- a/drivers/media/i2c/rdacm20.c
+++ b/drivers/media/i2c/rdacm20.c
@@ -360,7 +360,10 @@ static int __ov10635_write(struct rdacm20_device *dev, u16 reg, u8 val)
dev_dbg(dev->dev, "%s(0x%04x, 0x%02x)\n", __func__, reg, val);
ret = i2c_master_send(dev->sensor, buf, 3);
- return ret < 0 ? ret : 0;
+
+ if (ret < 0)
+ return ret;
+ return 0;
}
static int ov10635_write(struct rdacm20_device *dev, u16 reg, u8 val)
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/i2c/et8ek8/et8ek8_driver.c:255:13-14: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/i2c/et8ek8/et8ek8_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index e932d25ca7b3..c7984f90ae4d 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -252,7 +252,9 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
rval = i2c_transfer(client->adapter, msg, wcnt);
- return rval < 0 ? rval : 0;
+ if (rval < 0)
+ return rval;
+ return 0;
}
/*
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/usb/cx231xx/cx231xx-i2c.c:353:15-16: WARNING opportunity for min()
drivers/media/usb/cx231xx/cx231xx-i2c.c:262:15-16: WARNING opportunity for min()
drivers/media/usb/cx231xx/cx231xx-i2c.c:326:15-16: WARNING opportunity for min()
drivers/media/usb/cx231xx/cx231xx-i2c.c:176:15-16: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/cx231xx/cx231xx-i2c.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index c6659253c6fb..28de72856c90 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -173,7 +173,9 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
status = dev->cx231xx_send_usb_command(bus, &req_data);
}
- return status < 0 ? status : 0;
+ if (status < 0)
+ return status;
+ return 0;
}
/*
@@ -259,7 +261,9 @@ static int cx231xx_i2c_recv_bytes(struct i2c_adapter *i2c_adap,
status = dev->cx231xx_send_usb_command(bus, &req_data);
}
- return status < 0 ? status : 0;
+ if (status < 0)
+ return status;
+ return 0;
}
/*
@@ -323,7 +327,9 @@ static int cx231xx_i2c_recv_bytes_with_saddr(struct i2c_adapter *i2c_adap,
/* usb send command */
status = dev->cx231xx_send_usb_command(bus, &req_data);
- return status < 0 ? status : 0;
+ if (status < 0)
+ return status;
+ return 0;
}
/*
@@ -350,7 +356,9 @@ static int cx231xx_i2c_check_for_device(struct i2c_adapter *i2c_adap,
/* usb send command */
status = dev->cx231xx_send_usb_command(bus, &req_data);
- return status < 0 ? status : 0;
+ if (status < 0)
+ return status;
+ return 0;
}
/*
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/usb/hdpvr/hdpvr-control.c:41:12-13: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/hdpvr/hdpvr-control.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
index 37c53ab85b30..a1bb0231540c 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -38,7 +38,9 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf)
"config call request for value 0x%x returned %d\n", value,
ret);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/platform/qcom/venus/venc.c:611:12-13: WARNING opportunity for min()
drivers/media/platform/qcom/venus/venc.c:651:12-13: WARNING opportunity for min()
drivers/media/platform/qcom/venus/venc.c:629:12-13: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/qcom/venus/venc.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index de06e1712d09..285bc1b4d888 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -608,7 +608,9 @@ static int venc_pm_get(struct venus_inst *inst)
ret = pm_runtime_resume_and_get(dev);
mutex_unlock(&core->pm_lock);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
static int venc_pm_put(struct venus_inst *inst, bool autosuspend)
@@ -626,7 +628,9 @@ static int venc_pm_put(struct venus_inst *inst, bool autosuspend)
mutex_unlock(&core->pm_lock);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
static int venc_pm_get_put(struct venus_inst *inst)
@@ -648,7 +652,9 @@ static int venc_pm_get_put(struct venus_inst *inst)
error:
mutex_unlock(&core->pm_lock);
- return ret < 0 ? ret : 0;
+ if (ret < 0)
+ return ret;
+ return 0;
}
static void venc_pm_touch(struct venus_inst *inst)
--
2.44.0.683.g7961c838ac-goog
Reorder the branches a bit, so cocci stops complaining about the code.
drivers/media/platform/nvidia/tegra-vde/h264.c:645:20-21: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/nvidia/tegra-vde/h264.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/nvidia/tegra-vde/h264.c b/drivers/media/platform/nvidia/tegra-vde/h264.c
index 204e474d57f7..cfea5572a1b8 100644
--- a/drivers/media/platform/nvidia/tegra-vde/h264.c
+++ b/drivers/media/platform/nvidia/tegra-vde/h264.c
@@ -633,7 +633,9 @@ static int tegra_vde_decode_end(struct tegra_vde *vde)
timeout = wait_for_completion_interruptible_timeout(
&vde->decode_completion, msecs_to_jiffies(1000));
- if (timeout == 0) {
+ if (timeout < 0) {
+ ret = timeout;
+ } else if (timeout == 0) {
bsev_ptr = tegra_vde_readl(vde, vde->bsev, 0x10);
macroblocks_nb = tegra_vde_readl(vde, vde->sxe, 0xC8) & 0x1FFF;
read_bytes = bsev_ptr ? bsev_ptr - vde->bitstream_data_addr : 0;
@@ -642,8 +644,6 @@ static int tegra_vde_decode_end(struct tegra_vde *vde)
read_bytes, macroblocks_nb);
ret = -EIO;
- } else if (timeout < 0) {
- ret = timeout;
} else {
ret = 0;
}
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/pci/ttpci/budget-core.c:280:15-16: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/pci/ttpci/budget-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/pci/ttpci/budget-core.c b/drivers/media/pci/ttpci/budget-core.c
index 25f44c3eebf3..2d85ae1ef50b 100644
--- a/drivers/media/pci/ttpci/budget-core.c
+++ b/drivers/media/pci/ttpci/budget-core.c
@@ -277,7 +277,10 @@ static int ttpci_budget_debiwrite_nolock(struct budget *budget, u32 config,
saa7146_write(saa, MC2, (2 << 16) | 2);
result = saa7146_wait_for_debi_done(saa, nobusyloop);
- return result < 0 ? result : 0;
+
+ if (result < 0)
+ return result;
+ return 0;
}
int ttpci_budget_debiwrite(struct budget *budget, u32 config, int addr,
--
2.44.0.683.g7961c838ac-goog
This is a nop, but let cocci now that this is not a good candidate for
min()
drivers/media/radio/si4713/radio-usb-si4713.c:309:15-16: WARNING opportunity for min()
drivers/media/radio/si4713/radio-usb-si4713.c:360:15-16: WARNING opportunity for min()
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/radio/si4713/radio-usb-si4713.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/media/radio/si4713/radio-usb-si4713.c b/drivers/media/radio/si4713/radio-usb-si4713.c
index 2cf36c8abdde..0ad1bfe09004 100644
--- a/drivers/media/radio/si4713/radio-usb-si4713.c
+++ b/drivers/media/radio/si4713/radio-usb-si4713.c
@@ -306,7 +306,9 @@ static int send_command(struct si4713_usb_device *radio, u8 *payload, char *data
0x09, 0x21, 0x033f, 0, radio->buffer,
BUFFER_LENGTH, USB_TIMEOUT);
- return retval < 0 ? retval : 0;
+ if (retval < 0)
+ return retval;
+ return 0;
}
static int si4713_i2c_read(struct si4713_usb_device *radio, char *data, int len)
@@ -357,7 +359,9 @@ static int si4713_i2c_write(struct si4713_usb_device *radio, char *data, int len
data, len);
}
- return retval < 0 ? retval : 0;
+ if (retval < 0)
+ return retval;
+ return 0;
}
static int si4713_transfer(struct i2c_adapter *i2c_adapter,
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
> ---
> 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
Hi Laurent
On Mon, 15 Apr 2024 at 21:54, Laurent Pinchart
<[email protected]> 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 <[email protected]>
> > ---
> > 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
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
> <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
Dne ponedeljek, 15. april 2024 ob 21:34:24 GMT +2 je Ricardo Ribalda napisal(a):
> 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 <[email protected]>
Acked-by: Jernej Skrabec <[email protected]>
Best regards,
Jernej
> ---
> 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;
> goto error_clock_rate_exclusive;
> }
>
>
Quoting Ricardo Ribalda (2024-04-15 20:34:21)
> It makes the code slightly more clear and makes cocci incredibly happy:
>
> drivers/media/usb/uvc/uvc_ctrl.c:839:22-23: WARNING opportunity for max()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index a4a987913430..4b685f883e4d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -836,7 +836,7 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> while (1) {
> u8 byte = *data & mask;
> value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
> - bits -= 8 - (offset > 0 ? offset : 0);
> + bits -= 8 - max(offset, 0);
This looks semantically the same to me so:
Reviewed-by: Kieran Bingham <[email protected]>
> if (bits <= 0)
> break;
>
>
> --
> 2.44.0.683.g7961c838ac-goog
>
Hi Niklas
On Mon, 15 Apr 2024 at 22:33, Niklas Söderlund
<[email protected]> 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
> > <[email protected]> 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/[email protected]/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 <[email protected]>
> > > > ---
> > > > 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
> <[email protected]> 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
> > > <[email protected]> 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/[email protected]/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 <[email protected]>
> > > > > ---
> > > > > 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
--
Kind Regards,
Niklas Söderlund
Hello Ricardo,
On Mon, 15 Apr 2024 19:34:27 +0000
Ricardo Ribalda <[email protected]> wrote:
> Makes the code simpler and cocci happier:
>
> drivers/staging/media/tegra-video/tegra20.c:324:44-45: WARNING opportunity for swap()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Luca Ceresoli <[email protected]>
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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;
On (24/04/15 19:34), Ricardo Ribalda wrote:
> It makes the code slightly more clear and makes cocci incredibly happy:
>
> drivers/media/usb/uvc/uvc_ctrl.c:839:22-23: WARNING opportunity for max()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Sergey Senozhatsky <[email protected]>
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 <[email protected]>
I would find a hint for a variable change more appropriate in the patch subject
instead of the word “iterators”.
…
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
…
> @@ -2175,16 +2177,16 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> int ret;
>
> /* Find the extension unit. */
…
> + entity = NULL;
> + list_for_each_entry(iter, &chain->entities, chain) {
…
I suggest to move this assignment into the definition for the affected local variable.
By the way:
I see another source code adjustment opportunity in this function implementation.
https://elixir.bootlin.com/linux/v6.9-rc4/source/drivers/media/usb/uvc/uvc_ctrl.c#L2165
Can it be nicer to use labels “free_data” and “unlock” (instead of “done”)?
How do you think about to increase the application of scope-based resource management here?
Regards,
Markus
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/[email protected]/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 <[email protected]>
> > > > > > ---
> > > > > > 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
On Tue, Apr 16, 2024 at 09:03:36AM +0200, Markus Elfring wrote:
> I would find a hint for a variable change more appropriate in the patch subject
> instead of the word “iterators”.
>
>
> …
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> …
> > @@ -2175,16 +2177,16 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> > int ret;
> >
> > /* Find the extension unit. */
> …
> > + entity = NULL;
> > + list_for_each_entry(iter, &chain->entities, chain) {
> …
>
> I suggest to move this assignment into the definition for the affected local variable.
>
>
> By the way:
> I see another source code adjustment opportunity in this function implementation.
> https://elixir.bootlin.com/linux/v6.9-rc4/source/drivers/media/usb/uvc/uvc_ctrl.c#L2165
>
> Can it be nicer to use labels “free_data” and “unlock” (instead of “done”)?
> How do you think about to increase the application of scope-based resource management here?
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
On Mon, Apr 15, 2024 at 07:34:23PM +0000, Ricardo Ribalda wrote:
> platform_get_irq() already prints an error message.
>
> Found by cocci:
> drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c:444:3-10: line 444 is redundant because platform_get_irq() already prints an error
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
> index bce821eb71ce..c25027b0ca32 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
> @@ -439,11 +439,8 @@ static int dcmipp_probe(struct platform_device *pdev)
> "Could not get reset control\n");
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - if (irq != -EPROBE_DEFER)
> - dev_err(&pdev->dev, "Could not get irq\n");
> + if (irq <= 0)
> return irq ? irq : -ENXIO;
platform_get_irq() can never return zero so this should be written as:
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
There is a comment next to platform_get_irq() which documents this.
regards,
dan carpenter
On Mon, Apr 15, 2024 at 07:34:19PM +0000, Ricardo Ribalda wrote:
> chip_id is an unsigned number, it can never be < 0
>
> Fixes cocci check:
> drivers/media/dvb-frontends/stb0899_drv.c:1280:8-15: WARNING: Unsigned expression compared with zero: chip_id > 0
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/dvb-frontends/stb0899_drv.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c
> index 2f4d8fb400cd..222b5476ebfd 100644
> --- a/drivers/media/dvb-frontends/stb0899_drv.c
> +++ b/drivers/media/dvb-frontends/stb0899_drv.c
> @@ -1277,11 +1277,6 @@ static int stb0899_get_dev_id(struct stb0899_state *state)
> dprintk(state->verbose, FE_ERROR, 1, "Demodulator Core ID=[%s], Version=[%d]", (char *) &demod_str, demod_ver);
> CONVERT32(STB0899_READ_S2REG(STB0899_S2FEC, FEC_CORE_ID_REG), (char *)&fec_str);
> fec_ver = STB0899_READ_S2REG(STB0899_S2FEC, FEC_VER_ID_REG);
> - if (! (chip_id > 0)) {
This is not dead code. It's possible for chip_id to be equal to 0.
regards,
dan carpenter
> - dprintk(state->verbose, FE_ERROR, 1, "couldn't find a STB 0899");
> -
> - return -ENODEV;
> - }
> dprintk(state->verbose, FE_ERROR, 1, "FEC Core ID=[%s], Version=[%d]", (char*) &fec_str, fec_ver);
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
> }
>
On Mon, Apr 15, 2024 at 07:34:38PM +0000, Ricardo Ribalda wrote:
> us_per_frame does not fit in 32 bits. Use the appropriate div function.
>
Really? It's less than one frame per second?
regards,
dan carpenter
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/[email protected]/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 <[email protected]>
> > > > > > > ---
> > > > > > > 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 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 <[email protected]>
> ---
> 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
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);
}
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
>
--
Ricardo Ribalda
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
On Mon, Apr 15, 2024 at 07:34:42PM +0000, Ricardo Ribalda wrote:
> us_per_frame does not fit in u32
>
drivers/media/platform/qcom/venus/venc.c
391 static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
392 {
393 struct venus_inst *inst = to_inst(file);
394 struct v4l2_outputparm *out = &a->parm.output;
395 struct v4l2_fract *timeperframe = &out->timeperframe;
396 u64 us_per_frame, fps;
397
398 if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
399 a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
400 return -EINVAL;
401
402 memset(out->reserved, 0, sizeof(out->reserved));
403
404 if (!timeperframe->denominator)
405 timeperframe->denominator = inst->timeperframe.denominator;
406 if (!timeperframe->numerator)
407 timeperframe->numerator = inst->timeperframe.numerator;
408
409 out->capability = V4L2_CAP_TIMEPERFRAME;
410
411 us_per_frame = timeperframe->numerator * (u64)USEC_PER_SEC;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It looks like in some drivers this multiply can go over U32_MAX.
412 do_div(us_per_frame, timeperframe->denominator);
^^^^^^^^^^^^
But after this divide, then we're under 1,000,000 again. Otherwise the
FPS is zero. So maybe the right thing to do is:
inst->fps = USEC_PER_SEC / (u32)us_per_frame;
413
414 if (!us_per_frame)
415 return -EINVAL;
416
417 fps = (u64)USEC_PER_SEC;
418 do_div(fps, us_per_frame);
419
420 inst->timeperframe = *timeperframe;
421 inst->fps = fps;
422
423 return 0;
424 }
regards,
dan carpenter
Hi Ricardo,
Thanks a lot.
Reviewed-by: Benjamin Mugnier <[email protected]>
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 <[email protected]>
> ---
> 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;
>
--
Regards,
Benjamin
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.
--
Regards,
Laurent Pinchart
Hi Laurent
On Wed, 17 Apr 2024 at 17:51, Laurent Pinchart
<[email protected]> 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
--
Ricardo Ribalda
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.
--
Regards,
Laurent Pinchart
On Mon, Apr 15, 2024 at 07:34:21PM +0000, Ricardo Ribalda wrote:
> It makes the code slightly more clear and makes cocci incredibly happy:
>
> drivers/media/usb/uvc/uvc_ctrl.c:839:22-23: WARNING opportunity for max()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index a4a987913430..4b685f883e4d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -836,7 +836,7 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> while (1) {
> u8 byte = *data & mask;
> value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
> - bits -= 8 - (offset > 0 ? offset : 0);
> + bits -= 8 - max(offset, 0);
I don't think this really improve readability.
> if (bits <= 0)
> break;
>
--
Regards,
Laurent Pinchart
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 <[email protected]>
> ---
> 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;
>
--
Regards,
Laurent Pinchart
Hi Laurent
On Thu, 18 Apr 2024 at 12:53, Laurent Pinchart
<[email protected]> 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
--
Ricardo Ribalda
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.
--
Regards,
Laurent Pinchart
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.
regards,
dan carpenter
drivers/mtd/spi-nor/core.c:2896 spi_nor_late_init_params() warn: unnecessary div64_u64(): divisor = '2-255'
drivers/mtd/spi-nor/core.c:3409 spi_nor_set_mtd_eraseregions() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/hwmon/occ/common.c:467 occ_get_powr_avg() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/hwmon/occ/common.c:702 occ_store_caps_user() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/hwmon/scmi-hwmon.c:67 scmi_hwmon_scale() warn: unnecessary div64_u64(): divisor = '1,10'
drivers/infiniband/hw/cxgb4/device.c:160 wr_log_show() warn: unnecessary div64_u64(): divisor = '1000'
drivers/infiniband/hw/cxgb4/device.c:161 wr_log_show() warn: unnecessary div64_u64(): divisor = '1000'
drivers/clk/clk-versaclock7.c:743 vc7_get_apll_rate() warn: unnecessary div64_u64(): divisor = '2-31'
drivers/clk/clk-versaclock7.c:794 vc7_calc_fod_1st_stage_rate() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/clk/clk-versaclock7.c:812 vc7_calc_fod_2nd_stage_rate() warn: unnecessary div64_u64(): divisor = '2-u32max'
drivers/clk/clk-versaclock7.c:973 vc7_iod_recalc_rate() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/clk/clk-versaclock7.c:990 vc7_iod_round_rate() warn: unnecessary div64_u64(): divisor = '14-33554431'
drivers/clk/clk-versaclock7.c:1016 vc7_iod_set_rate() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/clk/clk-si570.c:263 si570_round_rate() warn: unnecessary div64_u64(): divisor = '2'
drivers/pci/setup-bus.c:1909 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/pci/setup-bus.c:1910 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/pci/setup-bus.c:1911 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/pci/setup-bus.c:1914 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/pci/setup-bus.c:1915 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/pci/setup-bus.c:1916 pci_bus_distribute_available_resources() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/tty/serial/serial_core.c:431 uart_update_timeout() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/ptp/ptp_qoriq.c:224 ptp_qoriq_adjfine() warn: unnecessary div64_u64(): divisor = '2'
drivers/ptp/ptp_dte.c:150 ptp_dte_adjfine() warn: unnecessary div64_u64(): divisor = '125000000'
drivers/ptp/ptp_dte.c:152 ptp_dte_adjfine() warn: unnecessary div64_u64(): divisor = '125000000'
drivers/gpu/drm/radeon/si_dpm.c:2200 si_calculate_power_efficiency_ratio() warn: unnecessary div64_u64(): divisor = '1000'
drivers/gpu/drm/radeon/si_dpm.c:2202 si_calculate_power_efficiency_ratio() warn: unnecessary div64_u64(): divisor = '1-4294836225'
drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c:356 rcar_mipi_dsi_pll_calc() warn: unnecessary div64_u64(): divisor = '0-255'
drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c:361 rcar_mipi_dsi_pll_calc() warn: unnecessary div64_u64(): divisor = '0-u16max'
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.c:2351 si_calculate_power_efficiency_ratio() warn: unnecessary div64_u64(): divisor = '1000'
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.c:2353 si_calculate_power_efficiency_ratio() warn: unnecessary div64_u64(): divisor = '1-4294836225'
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1673 amdgpu_ras_sysfs_badpages_read() warn: unnecessary div64_u64(): divisor = '28'
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1674 amdgpu_ras_sysfs_badpages_read() warn: unnecessary div64_u64(): divisor = '28'
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:592 populate_subvp_cmd_drr_info() warn: unnecessary div64_u64(): divisor = '2'
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dce110/dce110_hwseq.c:807 dce110_edp_power_control() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dce110/dce110_hwseq.c:812 dce110_edp_power_control() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dce110/dce110_hwseq.c:935 dce110_edp_wait_for_T12() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:765 edp_setup_psr() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:765 edp_setup_psr() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:800 kbps_to_peak_pbn() warn: unnecessary div64_u64(): divisor = '432000'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_psr.c:160 amdgpu_dm_psr_enable() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_psr.c:160 amdgpu_dm_psr_enable() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:106 calc_duration_in_us_from_refresh_in_uhz() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:117 calc_duration_in_us_from_v_total() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:132 mod_freesync_calc_v_total_from_refresh() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:135 mod_freesync_calc_v_total_from_refresh() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:135 mod_freesync_calc_v_total_from_refresh() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:169 calc_v_total_from_duration() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:169 calc_v_total_from_duration() warn: unnecessary div64_u64(): divisor = '1000'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:201 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '1000000'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:200 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '1000-4467765'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:207 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '1000'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:214 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '16666'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:245 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:245 update_v_total_for_static_ramp() warn: unnecessary div64_u64(): divisor = '1000'
drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.c:1004 mod_freesync_build_vrr_params() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/acpi/cppc_acpi.c:1855 cppc_perf_to_khz() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/acpi/cppc_acpi.c:1863 cppc_perf_to_khz() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/acpi/cppc_acpi.c:1885 cppc_khz_to_perf() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/leds/rgb/leds-qcom-lpg.c:458 lpg_calc_freq() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/leds/rgb/leds-qcom-lpg.c:464 lpg_calc_freq() warn: unnecessary div64_u64(): divisor = '1024'
drivers/md/dm-cache-policy-smq.c:1750 __smq_create() warn: unnecessary div64_u64(): divisor = '64-2097152'
drivers/md/dm-verity-fec.c:764 verity_fec_ctr() warn: unnecessary div64_u64(): divisor = '512-130560'
drivers/pwm/pwm-lpc32xx.c:48 lpc32xx_pwm_config() warn: unnecessary div64_u64(): divisor = 's32min-s32max'
drivers/pwm/pwm-bcm-iproc.c:138 iproc_pwmc_apply() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/pwm/pwm-bcm-iproc.c:140 iproc_pwmc_apply() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/pwm/pwm-sifive.c:95 pwm_sifive_update_clock() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/pwm/pwm-spear.c:98 spear_pwm_config() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/pwm/pwm-spear.c:100 spear_pwm_config() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/media/i2c/st-vgxy61.c:482 get_pixel_rate() warn: unnecessary div64_u64(): divisor = '0-255'
drivers/media/i2c/st-vgxy61.c:569 vgxy61_check_bw() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/media/i2c/ds90ub913.c:642 ub913_i2c_master_init() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/media/i2c/ds90ub913.c:643 ub913_i2c_master_init() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/media/i2c/ds90ub953.c:832 ub953_i2c_master_init() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/media/i2c/ds90ub953.c:833 ub953_i2c_master_init() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/media/platform/ti/vpe/sc.c:202 sc_config_scaler() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/media/dvb-core/dvb_demux.c:425 dvb_dmx_swfilter_packet() warn: unnecessary div64_u64(): divisor = '1024'
drivers/scsi/sym53c8xx_2/sym_hipd.c:749 sym_prepare_setting() warn: unnecessary div64_u64(): divisor = '40000-2560000'
drivers/video/fbdev/omap2/omapfb/dss/dsi.c:4773 dsi_vm_calc() warn: unnecessary div64_u64(): divisor = 's32min-s32max'
drivers/video/fbdev/omap2/omapfb/dss/dsi.c:4780 dsi_vm_calc() warn: unnecessary div64_u64(): divisor = 's32min-s32max'
drivers/cpufreq/cppc_cpufreq.c:124 cppc_scale_freq_workfn() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/pinctrl/ti/pinctrl-ti-iodelay.c:192 ti_iodelay_compute_dpe() warn: unnecessary div64_u64(): divisor = '176-34602480'
drivers/net/wireless/intel/iwlwifi/mvm/ptp.c:55 iwl_mvm_ptp_get_adj_time() warn: unnecessary div64_u64(): divisor = '1000'
drivers/net/ethernet/xilinx/xilinx_axienet_main.c:240 axienet_usec_to_timer() warn: unnecessary div64_u64(): divisor = '125000000'
drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:407 bnxt_get_target_cycles() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c:1262 hw_atl_b0_adj_params_get() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/net/ethernet/engleder/tsnep_selftests.c:164 delay_base_time() warn: unnecessary div64_u64(): divisor = '62500,100000,200000,333333,400000,411854,500000,1000000,1500000,1700000,2000000,6000000'
drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c:301 mlxsw_sp1_ptp_clock_init() warn: unnecessary div64_u64(): divisor = '3435819912'
drivers/net/ethernet/mellanox/mlx5/core/en/htb.c:263 mlx5e_htb_convert_rate() warn: unnecessary div64_u64(): divisor = '1-u32max'
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c:519 find_target_cycles() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c:966 mlx5_init_overflow_period() warn: unnecessary div64_u64(): divisor = '0-u32max'
drivers/net/ethernet/intel/ice/ice_ptp.c:1717 ice_ptp_cfg_clkout() warn: unnecessary div64_u64(): divisor = '1000000000'
drivers/base/arch_topology.c:406 topology_init_cpu_capacity_cppc() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iolatency.c:441 check_scale_change() warn: unnecessary div64_u64(): divisor = '100'
block/blk-iolatency.c:266 iolat_update_total_lat_avg() warn: unnecessary div64_u64(): divisor = '250000000'
block/blk-iolatency.c:236 latency_sum_ok() warn: unnecessary div64_u64(): divisor = '10'
block/blk-iolatency.c:955 iolatency_pd_stat() warn: unnecessary div64_u64(): divisor = '1000'
block/blk-iolatency.c:956 iolatency_pd_stat() warn: unnecessary div64_u64(): divisor = '1000000'
block/blk-iocost.c:706 abs_cost_to_cost() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:714 cost_to_abs_cost() warn: unnecessary div64_u64(): divisor = '65536'
block/blk-iocost.c:797 ioc_refresh_period_us() warn: unnecessary div64_u64(): divisor = '100'
block/blk-iocost.c:831 ioc_autop_idx() warn: unnecessary div64_u64(): divisor = '137438'
block/blk-iocost.c:943 ioc_refresh_params_disk() warn: unnecessary div64_u64(): divisor = '1000000'
block/blk-iocost.c:945 ioc_refresh_params_disk() warn: unnecessary div64_u64(): divisor = '1000000'
block/blk-iocost.c:1015 ioc_adjust_base_vrate() warn: unnecessary div64_u64(): divisor = '100'
block/blk-iocost.c:1018 ioc_adjust_base_vrate() warn: unnecessary div64_u64(): divisor = '100'
block/blk-iocost.c:1030 ioc_adjust_base_vrate() warn: unnecessary div64_u64(): divisor = '100'
block/blk-iocost.c:1365 iocg_kick_delay() warn: unnecessary div64_u64(): divisor = '1000000'
block/blk-iocost.c:1618 ioc_lat_stat() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:1762 hweight_after_donation() warn: unnecessary div64_u64(): divisor = '65536'
block/blk-iocost.c:1990 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-65536'
block/blk-iocost.c:1999 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '65536'
block/blk-iocost.c:2004 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2009 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2013 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2016 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2020 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2044 transfer_surpluses() warn: unnecessary div64_u64(): divisor = '0-u32max'
block/blk-iocost.c:2213 ioc_check_iocgs() warn: unnecessary div64_u64(): divisor = '65536'
block/blk-iocost.c:2823 ioc_rqos_done() warn: unnecessary div64_u64(): divisor = '137'
block/blk-iocost.c:3047 ioc_pd_stat() warn: unnecessary div64_u64(): divisor = '137438'
fs/bcachefs/tests.c:788 btree_perf_test_thread() warn: unnecessary div64_u64(): divisor = '0-u32max'
fs/bcachefs/alloc_background.c:2261 bch2_recalc_capacity() warn: unnecessary div64_u64(): divisor = '100'
fs/btrfs/block-group.c:2793 calculate_global_root_id() warn: unnecessary div64_u64(): divisor = '134217728,1073741824'
fs/nilfs2/the_nilfs.c:416 nilfs_max_segment_count() warn: unnecessary div64_u64(): divisor = '16-u32max'
fs/f2fs/gc.c:568 atgc_lookup_victim() warn: unnecessary div64_u64(): divisor = '0-u32max'
fs/f2fs/iostat.c:23 iostat_get_avg_bytes() warn: unnecessary div64_u64(): divisor = '0'
kernel/trace/trace_benchmark.c:107 trace_do_benchmark() warn: unnecessary div64_u64(): divisor = '0,2-u32max'
kernel/trace/trace_benchmark.c:129 trace_do_benchmark() warn: unnecessary div64_u64(): divisor = '1-u32max'
kernel/trace/trace_events_hist.c:701 hist_field_get_div_fn() warn: unnecessary div64_u64(): divisor = '1-1048576'
kernel/trace/trace_events_hist.c:5430 __get_percentage() warn: unnecessary div64_u64(): divisor = '10000'
kernel/dma/map_benchmark.c:82 map_benchmark_thread() warn: unnecessary div64_u64(): divisor = '100'
kernel/dma/map_benchmark.c:83 map_benchmark_thread() warn: unnecessary div64_u64(): divisor = '100'
kernel/fork.c:1011 set_max_threads() warn: unnecessary div64_u64(): divisor = '131072'
sound/usb/mixer_quirks.c:2353 snd_rme_current_freq_get() warn: unnecessary div64_u64(): divisor = '1-u32max'
sound/soc/sof/ipc4-mtrace.c:432 ipc4_mtrace_enable() warn: unnecessary div64_u64(): divisor = '1000'
sound/soc/sti/uniperif_player.c:182 uni_player_clk_set_rate() warn: unnecessary div64_u64(): divisor = '1000000'
net/netfilter/xt_hashlimit.c:498 user2credits() warn: unnecessary div64_u64(): divisor = '32000'
net/netfilter/xt_hashlimit.c:498 user2credits() warn: unnecessary div64_u64(): divisor = '31'
net/netfilter/xt_hashlimit.c:500 user2credits() warn: unnecessary div64_u64(): divisor = '10000,1000000'
net/ipv4/inetpeer.c:79 inet_initpeers() warn: unnecessary div64_u64(): divisor = '19200'
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> Use an API that resembles more the actual use of job_count.
>
> Found by cocci:
> drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:527:5-24: WARNING: atomic_dec_and_test variation before object free at line 541.
> drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:578:6-25: WARNING: atomic_dec_and_test variation before object free at line 581.
Same comment here as per the previous patch.
You should explain in terms of the memory ordering that refcount_t gives
so that the intention of the WARNING and the API change is well
communicated.
---
bod
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> Simplifies the code. Found by cocci:
>
> drivers/media/common/saa7146/saa7146_hlp.c:125:36-37: WARNING opportunity for min()
> drivers/media/common/saa7146/saa7146_hlp.c:154:41-42: WARNING opportunity for min()
> drivers/media/common/saa7146/saa7146_hlp.c:286:35-36: WARNING opportunity for min()
> drivers/media/common/saa7146/saa7146_hlp.c:289:35-36: WARNING opportunity for min()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Bryan O'Donoghue <[email protected]>
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> Use an API that resembles more the actual use of num_channels.
>
> Found by cocci:
> drivers/media/usb/s2255/s2255drv.c:2362:5-24: WARNING: atomic_dec_and_test variation before object free at line 2363.
> drivers/media/usb/s2255/s2255drv.c:1557:5-24: WARNING: atomic_dec_and_test variation before object free at line 1558.
Hmm, that commit log needs more detail.
"Convert from atomic_t to refcount_t because refcount_t has memory
ordering guarantees which atomic does not, hence the WARNING for the
free after the atomic dec."
Something like that.
I'll leave it up to yourself to decide if this warrants a Fixes:
I don't think so myself because the previous code doesn't seem to matter
to the decrement and free.
---
bod
On Tue, Apr 23, 2024 at 01:43:49PM +0100, Bryan O'Donoghue wrote:
> On 15/04/2024 20:34, Ricardo Ribalda wrote:
> > Use an API that resembles more the actual use of num_channels.
> >
> > Found by cocci:
> > drivers/media/usb/s2255/s2255drv.c:2362:5-24: WARNING: atomic_dec_and_test variation before object free at line 2363.
> > drivers/media/usb/s2255/s2255drv.c:1557:5-24: WARNING: atomic_dec_and_test variation before object free at line 1558.
>
> Hmm, that commit log needs more detail.
>
> "Convert from atomic_t to refcount_t because refcount_t has memory ordering
> guarantees which atomic does not, hence the WARNING for the free after the
> atomic dec."
>
The memory ordering rules are the same. They're basically identical.
The difference is that if you decrement a refcount_t past zero it will
trigger an error and refuse but atomic_t will merrily keep decrementing
forever. With refcount_t you can still have a use after free bug but
afterward the double free will trigger a warning.
There are time where people use atomic_t to get unique number and don't
care about wrapping so that's fine. But if it's reference counting then
use refcount_t.
There wouldn't be a Fixes tag in this case, because hopefully it's just
hardenning and not a bugfix.
regards,
dan carpenter
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> Simplifies the code.
>
> Found by cocci:
>
> drivers/media/dvb-frontends/drx39xyj/drxj.c:1447:23-24: WARNING opportunity for min()
> drivers/media/dvb-frontends/drx39xyj/drxj.c:1662:21-22: WARNING opportunity for min()
> drivers/media/dvb-frontends/drx39xyj/drxj.c:1685:24-25: WARNING opportunity for min()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Code is fine but, I think your commit log could use some love.
"Replace ternary assignments with min() to simplify and make the code
more readable."
Not super-important.
Reviewed-by: Bryan O'Donoghue <[email protected]>
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> Simplifies the code.
>
> Found by cocci:
> drivers/media/usb/stk1160/stk1160-video.c:133:12-13: WARNING opportunity for min()
> drivers/media/usb/stk1160/stk1160-video.c:176:13-14: WARNING opportunity for min()
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Show the commit log some love, then add
Reviewed-by: Bryan O'Donoghue <[email protected]>
For patches 16 through 19.
---
bod
On 15/04/2024 20:34, Ricardo Ribalda wrote:
> This is a nop, but let cocci now that this is not a good candidate for
*know
> min()
But I think you should change the commit log ->
"Rewrite ternary return assignment to mitigate the following cocci WARNING."
> drivers/media/platform/qcom/venus/vdec.c:672:12-13: WARNING opportunity for min()
> drivers/media/platform/qcom/venus/vdec.c:650:12-13: WARNING opportunity for min()
then
Reviewed-by: Bryan O'Donoghue <[email protected]>
---
bod
Hi Dan
On Tue, 23 Apr 2024 at 11:55, Dan Carpenter <[email protected]> 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!