Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()
From: Nicolas Dufresne <hidden>
Date: 2025-07-29 14:24:26
Also in:
linux-media, linux-patches, llvm, stable
Hi Nathan, Le mardi 15 juillet 2025 à 15:13 -0700, Nathan Chancellor a écrit :
A new warning in clang [1] points out a few places in s5p_mfc_cmd_v6.c
where an uninitialized variable is passed as a const pointer:
drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:45:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
45 | &h2r_args);
| ^~~~~~~~
drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:133:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
133 | &h2r_args);
| ^~~~~~~~
drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:148:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
148 | &h2r_args);
| ^~~~~~~~
The args parameter in s5p_mfc_cmd_host2risc_v6() is never actually used,
so just pass NULL to it in the places where h2r_args is currently
passed, clearing up the warning and not changing the functionality of
the code.
Cc: stable@vger.kernel.org
Fixes: f96f3cfa0bb8 ("[media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x")
Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441f19b319e [1]
Closes: https://github.com/ClangBuiltLinux/linux/issues/2103
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
From what I can tell, it seems like ->cmd_host2risc() is only ever
called from v6 code, which always passes NULL? It seems like it should
be possible to just drop .cmd_host2risc on the v5 side, then update
.cmd_host2risc to only take two parameters? If so, I can send a follow
up as a clean up, so that this can go back relatively conflict free.It seems so yes. For this specific patch, I would probably rename "args" to "__unused" to make the reading faster. But does not matter so much if you later remove it. Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
quoted hunk
--- .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c index 47bc3014b5d8..735471c50dbb 100644 --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c@@ -31,7 +31,6 @@ static int s5p_mfc_cmd_host2risc_v6(struct s5p_mfc_dev *dev, int cmd,static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev) { - struct s5p_mfc_cmd_args h2r_args; const struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv; int ret;@@ -41,33 +40,23 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6); mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6); - return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6, - &h2r_args); + return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6, NULL); } static int s5p_mfc_sleep_cmd_v6(struct s5p_mfc_dev *dev) { - struct s5p_mfc_cmd_args h2r_args; - - memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args)); - return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6, - &h2r_args); + return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6, NULL); } static int s5p_mfc_wakeup_cmd_v6(struct s5p_mfc_dev *dev) { - struct s5p_mfc_cmd_args h2r_args; - - memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args)); - return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6, - &h2r_args); + return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6, NULL); } /* Open a new instance and get its number */ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx) { struct s5p_mfc_dev *dev = ctx->dev; - struct s5p_mfc_cmd_args h2r_args; int codec_type; mfc_debug(2, "Requested codec mode: %d\n", ctx->codec_mode);@@ -130,14 +119,13 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)mfc_write(dev, 0, S5P_FIMV_D_CRC_CTRL_V6); /* no crc */ return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_OPEN_INSTANCE_V6, - &h2r_args); + NULL); } /* Close instance */ static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx) { struct s5p_mfc_dev *dev = ctx->dev; - struct s5p_mfc_cmd_args h2r_args; int ret = 0; dev->curr_ctx = ctx->num;@@ -145,7 +133,7 @@ static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)mfc_write(dev, ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6); ret = s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_CLOSE_INSTANCE_V6, - &h2r_args); + NULL); } else { ret = -EINVAL; } --- base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911 change-id: 20250715-media-s5p-mfc-fix-uninit-const-pointer-cbf944ae4b4b Best regards, -- Nathan Chancellor [off-list ref]
Attachments
- signature.asc [application/pgp-signature] 228 bytes