[PATCH] media: mediatek: vcodec: use = { } instead of memset()

[PATCH] media: mediatek: vcodec: use = { } instead of memset()

[Qianfeng Rong]

Based on testing and recommendations by David Lechner et al. [1][2],
using = { } to initialize a structure or array is the preferred way
to do this in the kernel.

This patch converts memset() to = { }, thereby:
- Eliminating the risk of sizeof() mismatches.
- Simplifying the code.

[1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/
[2]: https://lore.kernel.org/lkml/20250614151844.50524610@jic23-huawei/

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
 .../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c    |  3 +--
 .../mediatek/vcodec/decoder/vdec_vpu_if.c         | 12 ++++--------
 .../mediatek/vcodec/encoder/mtk_vcodec_enc.c      |  6 ++----
 .../mediatek/vcodec/encoder/venc_vpu_if.c         | 15 +++++----------
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
index eb3354192853..80554b2c26c0 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
@@ -548,10 +548,9 @@ static bool vp9_wait_dec_end(struct vdec_vp9_inst *inst)
 static struct vdec_vp9_inst *vp9_alloc_inst(struct mtk_vcodec_dec_ctx *ctx)
 {
 	int result;
-	struct mtk_vcodec_mem mem;
+	struct mtk_vcodec_mem mem = { };
 	struct vdec_vp9_inst *inst;
 
-	memset(&mem, 0, sizeof(mem));
 	mem.size = sizeof(struct vdec_vp9_inst);
 	result = mtk_vcodec_mem_alloc(ctx, &mem);
 	if (result)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index 145958206e38..d5e943f81c15 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -181,12 +181,11 @@ static int vcodec_vpu_send_msg(struct vdec_vpu_inst *vpu, void *msg, int len)
 
 static int vcodec_send_ap_ipi(struct vdec_vpu_inst *vpu, unsigned int msg_id)
 {
-	struct vdec_ap_ipi_cmd msg;
+	struct vdec_ap_ipi_cmd msg = { };
 	int err = 0;
 
 	mtk_vdec_debug(vpu->ctx, "+ id=%X", msg_id);
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_id = msg_id;
 	if (vpu->fw_abi_version < 2)
 		msg.vpu_inst_addr = vpu->inst_addr;
@@ -201,7 +200,7 @@ static int vcodec_send_ap_ipi(struct vdec_vpu_inst *vpu, unsigned int msg_id)
 
 int vpu_dec_init(struct vdec_vpu_inst *vpu)
 {
-	struct vdec_ap_ipi_init msg;
+	struct vdec_ap_ipi_init msg = { };
 	int err;
 
 	init_waitqueue_head(&vpu->wq);
@@ -225,7 +224,6 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
 		}
 	}
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_id = AP_IPIMSG_DEC_INIT;
 	msg.ap_inst_addr = (unsigned long)vpu;
 	msg.codec_type = vpu->codec_type;
@@ -245,7 +243,7 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
 
 int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t *data, unsigned int len)
 {
-	struct vdec_ap_ipi_dec_start msg;
+	struct vdec_ap_ipi_dec_start msg = { };
 	int i;
 	int err = 0;
 
@@ -254,7 +252,6 @@ int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t *data, unsigned int len)
 		return -EINVAL;
 	}
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_id = AP_IPIMSG_DEC_START;
 	if (vpu->fw_abi_version < 2)
 		msg.vpu_inst_addr = vpu->inst_addr;
@@ -273,7 +270,7 @@ int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t *data, unsigned int len)
 int vpu_dec_get_param(struct vdec_vpu_inst *vpu, uint32_t *data,
 		      unsigned int len, unsigned int param_type)
 {
-	struct vdec_ap_ipi_get_param msg;
+	struct vdec_ap_ipi_get_param msg = { };
 	int err;
 
 	if (len > ARRAY_SIZE(msg.data)) {
@@ -281,7 +278,6 @@ int vpu_dec_get_param(struct vdec_vpu_inst *vpu, uint32_t *data,
 		return -EINVAL;
 	}
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_id = AP_IPIMSG_DEC_GET_PARAM;
 	msg.inst_id = vpu->inst_id;
 	memcpy(msg.data, data, sizeof(unsigned int) * len);
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
index a01dc25a7699..bc6435a7543f 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
@@ -1064,7 +1064,7 @@ static int mtk_venc_encode_header(void *priv)
 
 static int mtk_venc_param_change(struct mtk_vcodec_enc_ctx *ctx)
 {
-	struct venc_enc_param enc_prm;
+	struct venc_enc_param enc_prm = { };
 	struct vb2_v4l2_buffer *vb2_v4l2 = v4l2_m2m_next_src_buf(ctx->m2m_ctx);
 	struct mtk_video_enc_buf *mtk_buf;
 	int ret = 0;
@@ -1075,7 +1075,6 @@ static int mtk_venc_param_change(struct mtk_vcodec_enc_ctx *ctx)
 
 	mtk_buf = container_of(vb2_v4l2, struct mtk_video_enc_buf, m2m_buf.vb);
 
-	memset(&enc_prm, 0, sizeof(enc_prm));
 	if (mtk_buf->param_change == MTK_ENCODE_PARAM_NONE)
 		return 0;
 
@@ -1138,7 +1137,7 @@ static void mtk_venc_worker(struct work_struct *work)
 	struct mtk_vcodec_enc_ctx *ctx = container_of(work, struct mtk_vcodec_enc_ctx,
 				    encode_work);
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
-	struct venc_frm_buf frm_buf;
+	struct venc_frm_buf frm_buf = { };
 	struct mtk_vcodec_mem bs_buf;
 	struct venc_done_result enc_result;
 	int ret, i;
@@ -1168,7 +1167,6 @@ static void mtk_venc_worker(struct work_struct *work)
 		return;
 	}
 
-	memset(&frm_buf, 0, sizeof(frm_buf));
 	for (i = 0; i < src_buf->vb2_buf.num_planes ; i++) {
 		frm_buf.fb_addr[i].dma_addr =
 				vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, i);
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
index 51bb7ee141b9..55627b71348d 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
@@ -132,7 +132,7 @@ static int vpu_enc_send_msg(struct venc_vpu_inst *vpu, void *msg,
 int vpu_enc_init(struct venc_vpu_inst *vpu)
 {
 	int status;
-	struct venc_ap_ipi_msg_init out;
+	struct venc_ap_ipi_msg_init out = { };
 
 	init_waitqueue_head(&vpu->wq_hd);
 	vpu->signaled = 0;
@@ -148,7 +148,6 @@ int vpu_enc_init(struct venc_vpu_inst *vpu)
 		return -EINVAL;
 	}
 
-	memset(&out, 0, sizeof(out));
 	out.msg_id = AP_IPIMSG_ENC_INIT;
 	out.venc_inst = (unsigned long)vpu;
 	if (vpu_enc_send_msg(vpu, &out, sizeof(out))) {
@@ -191,11 +190,10 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
 	size_t msg_size = is_ext ?
 		sizeof(struct venc_ap_ipi_msg_set_param_ext) :
 		sizeof(struct venc_ap_ipi_msg_set_param);
-	struct venc_ap_ipi_msg_set_param_ext out;
+	struct venc_ap_ipi_msg_set_param_ext out = { };
 
 	mtk_venc_debug(vpu->ctx, "id %d ->", id);
 
-	memset(&out, 0, sizeof(out));
 	out.base.msg_id = AP_IPIMSG_ENC_SET_PARAM;
 	out.base.vpu_inst_addr = vpu->inst_addr;
 	out.base.param_id = id;
@@ -258,11 +256,10 @@ static int vpu_enc_encode_32bits(struct venc_vpu_inst *vpu,
 	size_t msg_size = is_ext ?
 		sizeof(struct venc_ap_ipi_msg_enc_ext) :
 		sizeof(struct venc_ap_ipi_msg_enc);
-	struct venc_ap_ipi_msg_enc_ext out;
+	struct venc_ap_ipi_msg_enc_ext out = { };
 
 	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
 
-	memset(&out, 0, sizeof(out));
 	out.base.msg_id = AP_IPIMSG_ENC_ENCODE;
 	out.base.vpu_inst_addr = vpu->inst_addr;
 	out.base.bs_mode = bs_mode;
@@ -302,12 +299,11 @@ static int vpu_enc_encode_34bits(struct venc_vpu_inst *vpu,
 				 struct mtk_vcodec_mem *bs_buf,
 				 struct venc_frame_info *frame_info)
 {
-	struct venc_ap_ipi_msg_enc_ext_34 out;
+	struct venc_ap_ipi_msg_enc_ext_34 out = { };
 	size_t msg_size = sizeof(struct venc_ap_ipi_msg_enc_ext_34);
 
 	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
 
-	memset(&out, 0, sizeof(out));
 	out.msg_id = AP_IPIMSG_ENC_ENCODE;
 	out.vpu_inst_addr = vpu->inst_addr;
 	out.bs_mode = bs_mode;
@@ -367,9 +363,8 @@ int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
 
 int vpu_enc_deinit(struct venc_vpu_inst *vpu)
 {
-	struct venc_ap_ipi_msg_deinit out;
+	struct venc_ap_ipi_msg_deinit out = { };
 
-	memset(&out, 0, sizeof(out));
 	out.msg_id = AP_IPIMSG_ENC_DEINIT;
 	out.vpu_inst_addr = vpu->inst_addr;
 	if (vpu_enc_send_msg(vpu, &out, sizeof(out))) {
-- 
2.34.1

Re: [PATCH] media: mediatek: vcodec: use = { } instead of memset()

[Markus Elfring]

…
> This patch converts …

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94

Regards,
Markus

Re: [PATCH] media: mediatek: vcodec: use = { } instead of memset()

[Qianfeng Rong]

在 2025/8/5 21:06, Markus Elfring 写道:
> …
>> This patch converts …
> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94
I don't think your point is a problem here. Please focus more on the code
itself. If you have better suggestions for the code in my patch, please
let me know.

Best regards,
Qianfeng

Re: media: mediatek: vcodec: use = { } instead of memset()

[Markus Elfring]

>> …
>>> This patch converts …
>> See also once more:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94
> I don't think your point is a problem here. …

Will any contributors care more for the usage of imperative mood
also according to improved change descriptions?

Regards,
Markus

Re: [PATCH] media: mediatek: vcodec: use = { } instead of memset()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 9627 bytes --]

Hi,


Le dimanche 03 août 2025 à 21:55 +0800, Qianfeng Rong a écrit :
> Based on testing and recommendations by David Lechner et al. [1][2],
> using = { } to initialize a structure or array is the preferred way
> to do this in the kernel.
> 
> This patch converts memset() to = { }, thereby:
> - Eliminating the risk of sizeof() mismatches.
> - Simplifying the code.

Last month, Irui Wang sent an actual fix [0] for uninitialized data in this
driver. Your patch seems to be related, yet the previous fix is not covered and
this is not marked as a V2. Since this refactoring collide with an actual fix
that I'm waiting for a V2, I'd rather not take it and wait.

Any chances you can respin this with a second patches covering Irui's fix ?

cheers,
Nicolas


[0] https://patchwork.linuxtv.org/project/linux-media/patch/20250715081547.18076-1-irui.wang@mediatek.com/

> 
> [1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/
> [2]: https://lore.kernel.org/lkml/20250614151844.50524610@jic23-huawei/
> 
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> ---
>  .../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c    |  3 +--
>  .../mediatek/vcodec/decoder/vdec_vpu_if.c         | 12 ++++--------
>  .../mediatek/vcodec/encoder/mtk_vcodec_enc.c      |  6 ++----
>  .../mediatek/vcodec/encoder/venc_vpu_if.c         | 15 +++++----------
>  4 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> index eb3354192853..80554b2c26c0 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> @@ -548,10 +548,9 @@ static bool vp9_wait_dec_end(struct vdec_vp9_inst *inst)
>  static struct vdec_vp9_inst *vp9_alloc_inst(struct mtk_vcodec_dec_ctx *ctx)
>  {
>  	int result;
> -	struct mtk_vcodec_mem mem;
> +	struct mtk_vcodec_mem mem = { };
>  	struct vdec_vp9_inst *inst;
>  
> -	memset(&mem, 0, sizeof(mem));
>  	mem.size = sizeof(struct vdec_vp9_inst);
>  	result = mtk_vcodec_mem_alloc(ctx, &mem);
>  	if (result)
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> index 145958206e38..d5e943f81c15 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> @@ -181,12 +181,11 @@ static int vcodec_vpu_send_msg(struct vdec_vpu_inst
> *vpu, void *msg, int len)
>  
>  static int vcodec_send_ap_ipi(struct vdec_vpu_inst *vpu, unsigned int msg_id)
>  {
> -	struct vdec_ap_ipi_cmd msg;
> +	struct vdec_ap_ipi_cmd msg = { };
>  	int err = 0;
>  
>  	mtk_vdec_debug(vpu->ctx, "+ id=%X", msg_id);
>  
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_id = msg_id;
>  	if (vpu->fw_abi_version < 2)
>  		msg.vpu_inst_addr = vpu->inst_addr;
> @@ -201,7 +200,7 @@ static int vcodec_send_ap_ipi(struct vdec_vpu_inst *vpu,
> unsigned int msg_id)
>  
>  int vpu_dec_init(struct vdec_vpu_inst *vpu)
>  {
> -	struct vdec_ap_ipi_init msg;
> +	struct vdec_ap_ipi_init msg = { };
>  	int err;
>  
>  	init_waitqueue_head(&vpu->wq);
> @@ -225,7 +224,6 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
>  		}
>  	}
>  
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_id = AP_IPIMSG_DEC_INIT;
>  	msg.ap_inst_addr = (unsigned long)vpu;
>  	msg.codec_type = vpu->codec_type;
> @@ -245,7 +243,7 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
>  
>  int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t *data, unsigned int
> len)
>  {
> -	struct vdec_ap_ipi_dec_start msg;
> +	struct vdec_ap_ipi_dec_start msg = { };
>  	int i;
>  	int err = 0;
>  
> @@ -254,7 +252,6 @@ int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t
> *data, unsigned int len)
>  		return -EINVAL;
>  	}
>  
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_id = AP_IPIMSG_DEC_START;
>  	if (vpu->fw_abi_version < 2)
>  		msg.vpu_inst_addr = vpu->inst_addr;
> @@ -273,7 +270,7 @@ int vpu_dec_start(struct vdec_vpu_inst *vpu, uint32_t
> *data, unsigned int len)
>  int vpu_dec_get_param(struct vdec_vpu_inst *vpu, uint32_t *data,
>  		      unsigned int len, unsigned int param_type)
>  {
> -	struct vdec_ap_ipi_get_param msg;
> +	struct vdec_ap_ipi_get_param msg = { };
>  	int err;
>  
>  	if (len > ARRAY_SIZE(msg.data)) {
> @@ -281,7 +278,6 @@ int vpu_dec_get_param(struct vdec_vpu_inst *vpu, uint32_t
> *data,
>  		return -EINVAL;
>  	}
>  
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_id = AP_IPIMSG_DEC_GET_PARAM;
>  	msg.inst_id = vpu->inst_id;
>  	memcpy(msg.data, data, sizeof(unsigned int) * len);
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
> b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
> index a01dc25a7699..bc6435a7543f 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
> @@ -1064,7 +1064,7 @@ static int mtk_venc_encode_header(void *priv)
>  
>  static int mtk_venc_param_change(struct mtk_vcodec_enc_ctx *ctx)
>  {
> -	struct venc_enc_param enc_prm;
> +	struct venc_enc_param enc_prm = { };
>  	struct vb2_v4l2_buffer *vb2_v4l2 = v4l2_m2m_next_src_buf(ctx-
> >m2m_ctx);
>  	struct mtk_video_enc_buf *mtk_buf;
>  	int ret = 0;
> @@ -1075,7 +1075,6 @@ static int mtk_venc_param_change(struct
> mtk_vcodec_enc_ctx *ctx)
>  
>  	mtk_buf = container_of(vb2_v4l2, struct mtk_video_enc_buf,
> m2m_buf.vb);
>  
> -	memset(&enc_prm, 0, sizeof(enc_prm));
>  	if (mtk_buf->param_change == MTK_ENCODE_PARAM_NONE)
>  		return 0;
>  
> @@ -1138,7 +1137,7 @@ static void mtk_venc_worker(struct work_struct *work)
>  	struct mtk_vcodec_enc_ctx *ctx = container_of(work, struct
> mtk_vcodec_enc_ctx,
>  				    encode_work);
>  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> -	struct venc_frm_buf frm_buf;
> +	struct venc_frm_buf frm_buf = { };
>  	struct mtk_vcodec_mem bs_buf;
>  	struct venc_done_result enc_result;
>  	int ret, i;
> @@ -1168,7 +1167,6 @@ static void mtk_venc_worker(struct work_struct *work)
>  		return;
>  	}
>  
> -	memset(&frm_buf, 0, sizeof(frm_buf));
>  	for (i = 0; i < src_buf->vb2_buf.num_planes ; i++) {
>  		frm_buf.fb_addr[i].dma_addr =
>  				vb2_dma_contig_plane_dma_addr(&src_buf-
> >vb2_buf, i);
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> index 51bb7ee141b9..55627b71348d 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> @@ -132,7 +132,7 @@ static int vpu_enc_send_msg(struct venc_vpu_inst *vpu,
> void *msg,
>  int vpu_enc_init(struct venc_vpu_inst *vpu)
>  {
>  	int status;
> -	struct venc_ap_ipi_msg_init out;
> +	struct venc_ap_ipi_msg_init out = { };
>  
>  	init_waitqueue_head(&vpu->wq_hd);
>  	vpu->signaled = 0;
> @@ -148,7 +148,6 @@ int vpu_enc_init(struct venc_vpu_inst *vpu)
>  		return -EINVAL;
>  	}
>  
> -	memset(&out, 0, sizeof(out));
>  	out.msg_id = AP_IPIMSG_ENC_INIT;
>  	out.venc_inst = (unsigned long)vpu;
>  	if (vpu_enc_send_msg(vpu, &out, sizeof(out))) {
> @@ -191,11 +190,10 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
>  	size_t msg_size = is_ext ?
>  		sizeof(struct venc_ap_ipi_msg_set_param_ext) :
>  		sizeof(struct venc_ap_ipi_msg_set_param);
> -	struct venc_ap_ipi_msg_set_param_ext out;
> +	struct venc_ap_ipi_msg_set_param_ext out = { };
>  
>  	mtk_venc_debug(vpu->ctx, "id %d ->", id);
>  
> -	memset(&out, 0, sizeof(out));
>  	out.base.msg_id = AP_IPIMSG_ENC_SET_PARAM;
>  	out.base.vpu_inst_addr = vpu->inst_addr;
>  	out.base.param_id = id;
> @@ -258,11 +256,10 @@ static int vpu_enc_encode_32bits(struct venc_vpu_inst
> *vpu,
>  	size_t msg_size = is_ext ?
>  		sizeof(struct venc_ap_ipi_msg_enc_ext) :
>  		sizeof(struct venc_ap_ipi_msg_enc);
> -	struct venc_ap_ipi_msg_enc_ext out;
> +	struct venc_ap_ipi_msg_enc_ext out = { };
>  
>  	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
>  
> -	memset(&out, 0, sizeof(out));
>  	out.base.msg_id = AP_IPIMSG_ENC_ENCODE;
>  	out.base.vpu_inst_addr = vpu->inst_addr;
>  	out.base.bs_mode = bs_mode;
> @@ -302,12 +299,11 @@ static int vpu_enc_encode_34bits(struct venc_vpu_inst
> *vpu,
>  				 struct mtk_vcodec_mem *bs_buf,
>  				 struct venc_frame_info *frame_info)
>  {
> -	struct venc_ap_ipi_msg_enc_ext_34 out;
> +	struct venc_ap_ipi_msg_enc_ext_34 out = { };
>  	size_t msg_size = sizeof(struct venc_ap_ipi_msg_enc_ext_34);
>  
>  	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
>  
> -	memset(&out, 0, sizeof(out));
>  	out.msg_id = AP_IPIMSG_ENC_ENCODE;
>  	out.vpu_inst_addr = vpu->inst_addr;
>  	out.bs_mode = bs_mode;
> @@ -367,9 +363,8 @@ int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int
> bs_mode,
>  
>  int vpu_enc_deinit(struct venc_vpu_inst *vpu)
>  {
> -	struct venc_ap_ipi_msg_deinit out;
> +	struct venc_ap_ipi_msg_deinit out = { };
>  
> -	memset(&out, 0, sizeof(out));
>  	out.msg_id = AP_IPIMSG_ENC_DEINIT;
>  	out.vpu_inst_addr = vpu->inst_addr;
>  	if (vpu_enc_send_msg(vpu, &out, sizeof(out))) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: media: mediatek: vcodec: use = { } instead of memset()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

Le mardi 05 août 2025 à 15:32 +0200, Markus Elfring a écrit :
> > > …
> > > > This patch converts …
> > > See also once more:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94
> > I don't think your point is a problem here. …
> 
> Will any contributors care more for the usage of imperative mood
> also according to improved change descriptions?

Markus, its the way you review that does not work. I'm far from perfect, so I
don't normally give any lesson, but I've seen this this exact interaction too
often. Try suggesting a rephrase instead.

Qianfeng Rong, the suggestion is that:

   This patch converts memset() to = { }, thereby:
   
Can be rephrased:

   Converts memset() to = { }, thereby:

Also, some maintainers may prefer if you swap the two paraphs, so you get to the
point first, and give context in the remaining. Processing tones of patches is
improved if you don't have to read the entire message to understand what the
patch is doing at high level.

regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: media: mediatek: vcodec: use = { } instead of memset()

[Qianfeng Rong]

在 2025/8/30 2:57, Nicolas Dufresne 写道:
> Le mardi 05 août 2025 à 15:32 +0200, Markus Elfring a écrit :
>>>> …
>>>>> This patch converts …
>>>> See also once more:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94
>>> I don't think your point is a problem here. …
>> Will any contributors care more for the usage of imperative mood
>> also according to improved change descriptions?
> Markus, its the way you review that does not work. I'm far from perfect, so I
> don't normally give any lesson, but I've seen this this exact interaction too
> often. Try suggesting a rephrase instead.
>
> Qianfeng Rong, the suggestion is that:
>
>     This patch converts memset() to = { }, thereby:
>     
> Can be rephrased:
>
>     Converts memset() to = { }, thereby:
>
> Also, some maintainers may prefer if you swap the two paraphs, so you get to the
> point first, and give context in the remaining. Processing tones of patches is
> improved if you don't have to read the entire message to understand what the
> patch is doing at high level.


Describing patches and making them easily understandable often gives me
a headache.  Your suggestion has been particularly helpful, and it has
made me realize that this task is not as difficult as I imagined:
dividing the commit message into two sections. The first section
describes what the main changes are, and the second section provides
background information and motivation.

Thank you very much.

Best regards,
Qianfeng


>
> regards,
> Nicolas

Re: [PATCH] media: mediatek: vcodec: use = { } instead of memset()

[Qianfeng Rong]

在 2025/8/30 2:49, Nicolas Dufresne 写道:
> Hi,
>
>
> Le dimanche 03 août 2025 à 21:55 +0800, Qianfeng Rong a écrit :
>> Based on testing and recommendations by David Lechner et al. [1][2],
>> using = { } to initialize a structure or array is the preferred way
>> to do this in the kernel.
>>
>> This patch converts memset() to = { }, thereby:
>> - Eliminating the risk of sizeof() mismatches.
>> - Simplifying the code.
> Last month, Irui Wang sent an actual fix [0] for uninitialized data in this
> driver. Your patch seems to be related, yet the previous fix is not covered and
> this is not marked as a V2. Since this refactoring collide with an actual fix
> that I'm waiting for a V2, I'd rather not take it and wait.
>
> Any chances you can respin this with a second patches covering Irui's fix ?


Thanks for taking the time to explain!

If there are conflicts, please ignore my patch for now, as it might
not address the actual issue. After Irui's fix is merged, I can
update my patch if necessary.


Best regards,
Qianfeng