[PATCH] media: mediatek: vcodec: use = { } instead of memset()
Nicolas Dufresne
nicolas at ndufresne.ca
Fri Aug 29 11:49:47 PDT 2025
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 at 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))) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20250829/bb6b3f72/attachment.sig>
More information about the linux-arm-kernel
mailing list