CHECKED_RETURN: 4 case
REVERSE_INULL: 2 case
UNINIT: 6 case
UNUSED_VALUE: 1 case
Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
Signed-off-by: Ming Qian <[email protected]>
---
drivers/media/platform/amphion/vdec.c | 5 ++++-
drivers/media/platform/amphion/venc.c | 6 ++++--
drivers/media/platform/amphion/vpu_cmds.c | 5 +++--
drivers/media/platform/amphion/vpu_core.c | 2 ++
drivers/media/platform/amphion/vpu_dbg.c | 11 +++++++++--
drivers/media/platform/amphion/vpu_msgs.c | 12 ++++++------
6 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index eeb2ef72df5b..133d77d1ea0c 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -1019,6 +1019,7 @@ static int vdec_response_frame_abnormal(struct vpu_inst *inst)
{
struct vdec_t *vdec = inst->priv;
struct vpu_fs_info info;
+ int ret;
if (!vdec->req_frame_count)
return 0;
@@ -1026,7 +1027,9 @@ static int vdec_response_frame_abnormal(struct vpu_inst *inst)
memset(&info, 0, sizeof(info));
info.type = MEM_RES_FRAME;
info.tag = vdec->seq_tag + 0xf0;
- vpu_session_alloc_fs(inst, &info);
+ ret = vpu_session_alloc_fs(inst, &info);
+ if (ret)
+ return ret;
vdec->req_frame_count--;
return 0;
diff --git a/drivers/media/platform/amphion/venc.c b/drivers/media/platform/amphion/venc.c
index 58480e2755ec..4eb57d793a9c 100644
--- a/drivers/media/platform/amphion/venc.c
+++ b/drivers/media/platform/amphion/venc.c
@@ -268,7 +268,7 @@ static int venc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
{
struct vpu_inst *inst = to_inst(file);
struct venc_t *venc = inst->priv;
- struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
+ struct v4l2_fract *timeperframe;
if (!parm)
return -EINVAL;
@@ -279,6 +279,7 @@ static int venc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
if (!vpu_helper_check_type(inst, parm->type))
return -EINVAL;
+ timeperframe = &parm->parm.capture.timeperframe;
parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
parm->parm.capture.readbuffers = 0;
timeperframe->numerator = venc->params.frame_rate.numerator;
@@ -291,7 +292,7 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
{
struct vpu_inst *inst = to_inst(file);
struct venc_t *venc = inst->priv;
- struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
+ struct v4l2_fract *timeperframe;
unsigned long n, d;
if (!parm)
@@ -303,6 +304,7 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
if (!vpu_helper_check_type(inst, parm->type))
return -EINVAL;
+ timeperframe = &parm->parm.capture.timeperframe;
if (!timeperframe->numerator)
timeperframe->numerator = venc->params.frame_rate.numerator;
if (!timeperframe->denominator)
diff --git a/drivers/media/platform/amphion/vpu_cmds.c b/drivers/media/platform/amphion/vpu_cmds.c
index 647d94554fb5..235b71398d40 100644
--- a/drivers/media/platform/amphion/vpu_cmds.c
+++ b/drivers/media/platform/amphion/vpu_cmds.c
@@ -306,7 +306,8 @@ static void vpu_core_keep_active(struct vpu_core *core)
dev_dbg(core->dev, "try to wake up\n");
mutex_lock(&core->cmd_lock);
- vpu_cmd_send(core, &pkt);
+ if (vpu_cmd_send(core, &pkt))
+ dev_err(core->dev, "fail to keep active\n");
mutex_unlock(&core->cmd_lock);
}
@@ -314,7 +315,7 @@ static int vpu_session_send_cmd(struct vpu_inst *inst, u32 id, void *data)
{
unsigned long key;
int sync = false;
- int ret = -EINVAL;
+ int ret;
if (inst->id < 0)
return -EINVAL;
diff --git a/drivers/media/platform/amphion/vpu_core.c b/drivers/media/platform/amphion/vpu_core.c
index 43d85a54268b..6f054700d5db 100644
--- a/drivers/media/platform/amphion/vpu_core.c
+++ b/drivers/media/platform/amphion/vpu_core.c
@@ -88,6 +88,8 @@ static int vpu_core_boot_done(struct vpu_core *core)
core->supported_instance_count = min(core->supported_instance_count, count);
}
+ if (core->supported_instance_count >= BITS_PER_TYPE(core->instance_mask))
+ core->supported_instance_count = BITS_PER_TYPE(core->instance_mask);
core->fw_version = fw_version;
vpu_core_set_state(core, VPU_CORE_ACTIVE);
diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c
index adc523b95061..982c2c777484 100644
--- a/drivers/media/platform/amphion/vpu_dbg.c
+++ b/drivers/media/platform/amphion/vpu_dbg.c
@@ -50,6 +50,13 @@ static char *vpu_stat_name[] = {
[VPU_BUF_STATE_ERROR] = "error",
};
+static inline const char *to_vpu_stat_name(int state)
+{
+ if (state <= VPU_BUF_STATE_ERROR)
+ return vpu_stat_name[state];
+ return "unknown";
+}
+
static int vpu_dbg_instance(struct seq_file *s, void *data)
{
struct vpu_inst *inst = s->private;
@@ -141,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
num = scnprintf(str, sizeof(str),
"output [%2d] state = %10s, %8s\n",
i, vb2_stat_name[vb->state],
- vpu_stat_name[vpu_get_buffer_state(vbuf)]);
+ to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
if (seq_write(s, str, num))
return 0;
}
@@ -156,7 +163,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
num = scnprintf(str, sizeof(str),
"capture[%2d] state = %10s, %8s\n",
i, vb2_stat_name[vb->state],
- vpu_stat_name[vpu_get_buffer_state(vbuf)]);
+ to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
if (seq_write(s, str, num))
return 0;
}
diff --git a/drivers/media/platform/amphion/vpu_msgs.c b/drivers/media/platform/amphion/vpu_msgs.c
index f9eb488d1b5e..d0ead051f7d1 100644
--- a/drivers/media/platform/amphion/vpu_msgs.c
+++ b/drivers/media/platform/amphion/vpu_msgs.c
@@ -32,7 +32,7 @@ static void vpu_session_handle_start_done(struct vpu_inst *inst, struct vpu_rpc_
static void vpu_session_handle_mem_request(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
{
- struct vpu_pkt_mem_req_data req_data;
+ struct vpu_pkt_mem_req_data req_data = { 0 };
vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&req_data);
vpu_trace(inst->dev, "[%d] %d:%d %d:%d %d:%d\n",
@@ -80,7 +80,7 @@ static void vpu_session_handle_resolution_change(struct vpu_inst *inst, struct v
static void vpu_session_handle_enc_frame_done(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
{
- struct vpu_enc_pic_info info;
+ struct vpu_enc_pic_info info = { 0 };
vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
dev_dbg(inst->dev, "[%d] frame id = %d, wptr = 0x%x, size = %d\n",
@@ -90,7 +90,7 @@ static void vpu_session_handle_enc_frame_done(struct vpu_inst *inst, struct vpu_
static void vpu_session_handle_frame_request(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
{
- struct vpu_fs_info fs;
+ struct vpu_fs_info fs = { 0 };
vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
call_void_vop(inst, event_notify, VPU_MSG_ID_FRAME_REQ, &fs);
@@ -107,7 +107,7 @@ static void vpu_session_handle_frame_release(struct vpu_inst *inst, struct vpu_r
info.type = inst->out_format.type;
call_void_vop(inst, buf_done, &info);
} else if (inst->core->type == VPU_CORE_TYPE_DEC) {
- struct vpu_fs_info fs;
+ struct vpu_fs_info fs = { 0 };
vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
call_void_vop(inst, event_notify, VPU_MSG_ID_FRAME_RELEASE, &fs);
@@ -122,7 +122,7 @@ static void vpu_session_handle_input_done(struct vpu_inst *inst, struct vpu_rpc_
static void vpu_session_handle_pic_decoded(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
{
- struct vpu_dec_pic_info info;
+ struct vpu_dec_pic_info info = { 0 };
vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
call_void_vop(inst, get_one_frame, &info);
@@ -130,7 +130,7 @@ static void vpu_session_handle_pic_decoded(struct vpu_inst *inst, struct vpu_rpc
static void vpu_session_handle_pic_done(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
{
- struct vpu_dec_pic_info info;
+ struct vpu_dec_pic_info info = { 0 };
struct vpu_frame_info frame;
memset(&frame, 0, sizeof(frame));
--
2.38.1
Hi Ming,
Le lundi 17 juillet 2023 à 15:40 +0800, Ming Qian a écrit :
> CHECKED_RETURN: 4 case
> REVERSE_INULL: 2 case
> UNINIT: 6 case
> UNUSED_VALUE: 1 case
>
> Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
> Signed-off-by: Ming Qian <[email protected]>
> ---
> drivers/media/platform/amphion/vdec.c | 5 ++++-
> drivers/media/platform/amphion/venc.c | 6 ++++--
> drivers/media/platform/amphion/vpu_cmds.c | 5 +++--
> drivers/media/platform/amphion/vpu_core.c | 2 ++
> drivers/media/platform/amphion/vpu_dbg.c | 11 +++++++++--
> drivers/media/platform/amphion/vpu_msgs.c | 12 ++++++------
> 6 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
> index eeb2ef72df5b..133d77d1ea0c 100644
> --- a/drivers/media/platform/amphion/vdec.c
> +++ b/drivers/media/platform/amphion/vdec.c
> @@ -1019,6 +1019,7 @@ static int vdec_response_frame_abnormal(struct vpu_inst *inst)
> {
> struct vdec_t *vdec = inst->priv;
> struct vpu_fs_info info;
> + int ret;
>
> if (!vdec->req_frame_count)
> return 0;
> @@ -1026,7 +1027,9 @@ static int vdec_response_frame_abnormal(struct vpu_inst *inst)
> memset(&info, 0, sizeof(info));
> info.type = MEM_RES_FRAME;
> info.tag = vdec->seq_tag + 0xf0;
> - vpu_session_alloc_fs(inst, &info);
> + ret = vpu_session_alloc_fs(inst, &info);
> + if (ret)
> + return ret;
> vdec->req_frame_count--;
>
> return 0;
> diff --git a/drivers/media/platform/amphion/venc.c b/drivers/media/platform/amphion/venc.c
> index 58480e2755ec..4eb57d793a9c 100644
> --- a/drivers/media/platform/amphion/venc.c
> +++ b/drivers/media/platform/amphion/venc.c
> @@ -268,7 +268,7 @@ static int venc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
> {
> struct vpu_inst *inst = to_inst(file);
> struct venc_t *venc = inst->priv;
> - struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
> + struct v4l2_fract *timeperframe;
Could be just me, but I feel I'm missing some context to understand why this
change. Perhaps the commit message could be improved ?
All other changes looks like improvement to me, so with a good explanation on
this one (and the change seems to be equivalent), you can add:
Reviewed-by: Nicolas Dufresne <[email protected]>
>
> if (!parm)
> return -EINVAL;
> @@ -279,6 +279,7 @@ static int venc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
> if (!vpu_helper_check_type(inst, parm->type))
> return -EINVAL;
>
> + timeperframe = &parm->parm.capture.timeperframe;
> parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> parm->parm.capture.readbuffers = 0;
> timeperframe->numerator = venc->params.frame_rate.numerator;
> @@ -291,7 +292,7 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
> {
> struct vpu_inst *inst = to_inst(file);
> struct venc_t *venc = inst->priv;
> - struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
> + struct v4l2_fract *timeperframe;
> unsigned long n, d;
>
> if (!parm)
> @@ -303,6 +304,7 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *parm
> if (!vpu_helper_check_type(inst, parm->type))
> return -EINVAL;
>
> + timeperframe = &parm->parm.capture.timeperframe;
> if (!timeperframe->numerator)
> timeperframe->numerator = venc->params.frame_rate.numerator;
> if (!timeperframe->denominator)
> diff --git a/drivers/media/platform/amphion/vpu_cmds.c b/drivers/media/platform/amphion/vpu_cmds.c
> index 647d94554fb5..235b71398d40 100644
> --- a/drivers/media/platform/amphion/vpu_cmds.c
> +++ b/drivers/media/platform/amphion/vpu_cmds.c
> @@ -306,7 +306,8 @@ static void vpu_core_keep_active(struct vpu_core *core)
>
> dev_dbg(core->dev, "try to wake up\n");
> mutex_lock(&core->cmd_lock);
> - vpu_cmd_send(core, &pkt);
> + if (vpu_cmd_send(core, &pkt))
> + dev_err(core->dev, "fail to keep active\n");
> mutex_unlock(&core->cmd_lock);
> }
>
> @@ -314,7 +315,7 @@ static int vpu_session_send_cmd(struct vpu_inst *inst, u32 id, void *data)
> {
> unsigned long key;
> int sync = false;
> - int ret = -EINVAL;
> + int ret;
>
> if (inst->id < 0)
> return -EINVAL;
> diff --git a/drivers/media/platform/amphion/vpu_core.c b/drivers/media/platform/amphion/vpu_core.c
> index 43d85a54268b..6f054700d5db 100644
> --- a/drivers/media/platform/amphion/vpu_core.c
> +++ b/drivers/media/platform/amphion/vpu_core.c
> @@ -88,6 +88,8 @@ static int vpu_core_boot_done(struct vpu_core *core)
>
> core->supported_instance_count = min(core->supported_instance_count, count);
> }
> + if (core->supported_instance_count >= BITS_PER_TYPE(core->instance_mask))
> + core->supported_instance_count = BITS_PER_TYPE(core->instance_mask);
> core->fw_version = fw_version;
> vpu_core_set_state(core, VPU_CORE_ACTIVE);
>
> diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c
> index adc523b95061..982c2c777484 100644
> --- a/drivers/media/platform/amphion/vpu_dbg.c
> +++ b/drivers/media/platform/amphion/vpu_dbg.c
> @@ -50,6 +50,13 @@ static char *vpu_stat_name[] = {
> [VPU_BUF_STATE_ERROR] = "error",
> };
>
> +static inline const char *to_vpu_stat_name(int state)
> +{
> + if (state <= VPU_BUF_STATE_ERROR)
> + return vpu_stat_name[state];
> + return "unknown";
> +}
> +
> static int vpu_dbg_instance(struct seq_file *s, void *data)
> {
> struct vpu_inst *inst = s->private;
> @@ -141,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
> num = scnprintf(str, sizeof(str),
> "output [%2d] state = %10s, %8s\n",
> i, vb2_stat_name[vb->state],
> - vpu_stat_name[vpu_get_buffer_state(vbuf)]);
> + to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
> if (seq_write(s, str, num))
> return 0;
> }
> @@ -156,7 +163,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
> num = scnprintf(str, sizeof(str),
> "capture[%2d] state = %10s, %8s\n",
> i, vb2_stat_name[vb->state],
> - vpu_stat_name[vpu_get_buffer_state(vbuf)]);
> + to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
> if (seq_write(s, str, num))
> return 0;
> }
> diff --git a/drivers/media/platform/amphion/vpu_msgs.c b/drivers/media/platform/amphion/vpu_msgs.c
> index f9eb488d1b5e..d0ead051f7d1 100644
> --- a/drivers/media/platform/amphion/vpu_msgs.c
> +++ b/drivers/media/platform/amphion/vpu_msgs.c
> @@ -32,7 +32,7 @@ static void vpu_session_handle_start_done(struct vpu_inst *inst, struct vpu_rpc_
>
> static void vpu_session_handle_mem_request(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> {
> - struct vpu_pkt_mem_req_data req_data;
> + struct vpu_pkt_mem_req_data req_data = { 0 };
>
> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&req_data);
> vpu_trace(inst->dev, "[%d] %d:%d %d:%d %d:%d\n",
> @@ -80,7 +80,7 @@ static void vpu_session_handle_resolution_change(struct vpu_inst *inst, struct v
>
> static void vpu_session_handle_enc_frame_done(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> {
> - struct vpu_enc_pic_info info;
> + struct vpu_enc_pic_info info = { 0 };
>
> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
> dev_dbg(inst->dev, "[%d] frame id = %d, wptr = 0x%x, size = %d\n",
> @@ -90,7 +90,7 @@ static void vpu_session_handle_enc_frame_done(struct vpu_inst *inst, struct vpu_
>
> static void vpu_session_handle_frame_request(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> {
> - struct vpu_fs_info fs;
> + struct vpu_fs_info fs = { 0 };
>
> vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
> call_void_vop(inst, event_notify, VPU_MSG_ID_FRAME_REQ, &fs);
> @@ -107,7 +107,7 @@ static void vpu_session_handle_frame_release(struct vpu_inst *inst, struct vpu_r
> info.type = inst->out_format.type;
> call_void_vop(inst, buf_done, &info);
> } else if (inst->core->type == VPU_CORE_TYPE_DEC) {
> - struct vpu_fs_info fs;
> + struct vpu_fs_info fs = { 0 };
>
> vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
> call_void_vop(inst, event_notify, VPU_MSG_ID_FRAME_RELEASE, &fs);
> @@ -122,7 +122,7 @@ static void vpu_session_handle_input_done(struct vpu_inst *inst, struct vpu_rpc_
>
> static void vpu_session_handle_pic_decoded(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> {
> - struct vpu_dec_pic_info info;
> + struct vpu_dec_pic_info info = { 0 };
>
> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
> call_void_vop(inst, get_one_frame, &info);
> @@ -130,7 +130,7 @@ static void vpu_session_handle_pic_decoded(struct vpu_inst *inst, struct vpu_rpc
>
> static void vpu_session_handle_pic_done(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> {
> - struct vpu_dec_pic_info info;
> + struct vpu_dec_pic_info info = { 0 };
> struct vpu_frame_info frame;
>
> memset(&frame, 0, sizeof(frame));
>From: Nicolas Dufresne <[email protected]>
>Sent: 2023年7月17日 22:55
>To: Ming Qian <[email protected]>; [email protected]; hverkuil-
>[email protected]
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; dl-linux-imx <linux-
>[email protected]>; X.H. Bao <[email protected]>; Eagle Zhou
><[email protected]>; Tao Jiang <[email protected]>; linux-
>[email protected]; [email protected]; linux-arm-
>[email protected]
>Subject: [EXT] Re: [PATCH] media: amphion: fix some issues reported by
>coverity
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>Hi Ming,
>
>Le lundi 17 juillet 2023 à 15:40 +0800, Ming Qian a écrit :
>> CHECKED_RETURN: 4 case
>> REVERSE_INULL: 2 case
>> UNINIT: 6 case
>> UNUSED_VALUE: 1 case
>>
>> Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
>> Signed-off-by: Ming Qian <[email protected]>
>> ---
>> drivers/media/platform/amphion/vdec.c | 5 ++++-
>> drivers/media/platform/amphion/venc.c | 6 ++++--
>> drivers/media/platform/amphion/vpu_cmds.c | 5 +++--
>> drivers/media/platform/amphion/vpu_core.c | 2 ++
>> drivers/media/platform/amphion/vpu_dbg.c | 11 +++++++++--
>> drivers/media/platform/amphion/vpu_msgs.c | 12 ++++++------
>> 6 files changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/platform/amphion/vdec.c
>> b/drivers/media/platform/amphion/vdec.c
>> index eeb2ef72df5b..133d77d1ea0c 100644
>> --- a/drivers/media/platform/amphion/vdec.c
>> +++ b/drivers/media/platform/amphion/vdec.c
>> @@ -1019,6 +1019,7 @@ static int vdec_response_frame_abnormal(struct
>> vpu_inst *inst) {
>> struct vdec_t *vdec = inst->priv;
>> struct vpu_fs_info info;
>> + int ret;
>>
>> if (!vdec->req_frame_count)
>> return 0;
>> @@ -1026,7 +1027,9 @@ static int vdec_response_frame_abnormal(struct
>vpu_inst *inst)
>> memset(&info, 0, sizeof(info));
>> info.type = MEM_RES_FRAME;
>> info.tag = vdec->seq_tag + 0xf0;
>> - vpu_session_alloc_fs(inst, &info);
>> + ret = vpu_session_alloc_fs(inst, &info);
>> + if (ret)
>> + return ret;
>> vdec->req_frame_count--;
>>
>> return 0;
>> diff --git a/drivers/media/platform/amphion/venc.c
>> b/drivers/media/platform/amphion/venc.c
>> index 58480e2755ec..4eb57d793a9c 100644
>> --- a/drivers/media/platform/amphion/venc.c
>> +++ b/drivers/media/platform/amphion/venc.c
>> @@ -268,7 +268,7 @@ static int venc_g_parm(struct file *file, void
>> *fh, struct v4l2_streamparm *parm {
>> struct vpu_inst *inst = to_inst(file);
>> struct venc_t *venc = inst->priv;
>> - struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
>> + struct v4l2_fract *timeperframe;
>
>Could be just me, but I feel I'm missing some context to understand why this
>change. Perhaps the commit message could be improved ?
>
>All other changes looks like improvement to me, so with a good explanation
>on this one (and the change seems to be equivalent), you can add:
>
>Reviewed-by: Nicolas Dufresne <[email protected]>
>
Hi Nicolas,
The Coverity scan report a REVERSE_INULL issue here, that directly dereferencing pointer "param", before Null-checking "parm".
I'll split this patch into several patches, one topic one patch.
Ming
>>
>> if (!parm)
>> return -EINVAL;
>> @@ -279,6 +279,7 @@ static int venc_g_parm(struct file *file, void *fh,
>struct v4l2_streamparm *parm
>> if (!vpu_helper_check_type(inst, parm->type))
>> return -EINVAL;
>>
>> + timeperframe = &parm->parm.capture.timeperframe;
>> parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>> parm->parm.capture.readbuffers = 0;
>> timeperframe->numerator = venc->params.frame_rate.numerator;
>> @@ -291,7 +292,7 @@ static int venc_s_parm(struct file *file, void
>> *fh, struct v4l2_streamparm *parm {
>> struct vpu_inst *inst = to_inst(file);
>> struct venc_t *venc = inst->priv;
>> - struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
>> + struct v4l2_fract *timeperframe;
>> unsigned long n, d;
>>
>> if (!parm)
>> @@ -303,6 +304,7 @@ static int venc_s_parm(struct file *file, void *fh,
>struct v4l2_streamparm *parm
>> if (!vpu_helper_check_type(inst, parm->type))
>> return -EINVAL;
>>
>> + timeperframe = &parm->parm.capture.timeperframe;
>> if (!timeperframe->numerator)
>> timeperframe->numerator = venc->params.frame_rate.numerator;
>> if (!timeperframe->denominator)
>> diff --git a/drivers/media/platform/amphion/vpu_cmds.c
>> b/drivers/media/platform/amphion/vpu_cmds.c
>> index 647d94554fb5..235b71398d40 100644
>> --- a/drivers/media/platform/amphion/vpu_cmds.c
>> +++ b/drivers/media/platform/amphion/vpu_cmds.c
>> @@ -306,7 +306,8 @@ static void vpu_core_keep_active(struct vpu_core
>> *core)
>>
>> dev_dbg(core->dev, "try to wake up\n");
>> mutex_lock(&core->cmd_lock);
>> - vpu_cmd_send(core, &pkt);
>> + if (vpu_cmd_send(core, &pkt))
>> + dev_err(core->dev, "fail to keep active\n");
>> mutex_unlock(&core->cmd_lock);
>> }
>>
>> @@ -314,7 +315,7 @@ static int vpu_session_send_cmd(struct vpu_inst
>> *inst, u32 id, void *data) {
>> unsigned long key;
>> int sync = false;
>> - int ret = -EINVAL;
>> + int ret;
>>
>> if (inst->id < 0)
>> return -EINVAL;
>> diff --git a/drivers/media/platform/amphion/vpu_core.c
>> b/drivers/media/platform/amphion/vpu_core.c
>> index 43d85a54268b..6f054700d5db 100644
>> --- a/drivers/media/platform/amphion/vpu_core.c
>> +++ b/drivers/media/platform/amphion/vpu_core.c
>> @@ -88,6 +88,8 @@ static int vpu_core_boot_done(struct vpu_core *core)
>>
>> core->supported_instance_count = min(core-
>>supported_instance_count, count);
>> }
>> + if (core->supported_instance_count >= BITS_PER_TYPE(core-
>>instance_mask))
>> + core->supported_instance_count =
>> + BITS_PER_TYPE(core->instance_mask);
>> core->fw_version = fw_version;
>> vpu_core_set_state(core, VPU_CORE_ACTIVE);
>>
>> diff --git a/drivers/media/platform/amphion/vpu_dbg.c
>> b/drivers/media/platform/amphion/vpu_dbg.c
>> index adc523b95061..982c2c777484 100644
>> --- a/drivers/media/platform/amphion/vpu_dbg.c
>> +++ b/drivers/media/platform/amphion/vpu_dbg.c
>> @@ -50,6 +50,13 @@ static char *vpu_stat_name[] = {
>> [VPU_BUF_STATE_ERROR] = "error", };
>>
>> +static inline const char *to_vpu_stat_name(int state) {
>> + if (state <= VPU_BUF_STATE_ERROR)
>> + return vpu_stat_name[state];
>> + return "unknown";
>> +}
>> +
>> static int vpu_dbg_instance(struct seq_file *s, void *data) {
>> struct vpu_inst *inst = s->private; @@ -141,7 +148,7 @@ static
>> int vpu_dbg_instance(struct seq_file *s, void *data)
>> num = scnprintf(str, sizeof(str),
>> "output [%2d] state = %10s, %8s\n",
>> i, vb2_stat_name[vb->state],
>> - vpu_stat_name[vpu_get_buffer_state(vbuf)]);
>> +
>> + to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
>> if (seq_write(s, str, num))
>> return 0;
>> }
>> @@ -156,7 +163,7 @@ static int vpu_dbg_instance(struct seq_file *s, void
>*data)
>> num = scnprintf(str, sizeof(str),
>> "capture[%2d] state = %10s, %8s\n",
>> i, vb2_stat_name[vb->state],
>> - vpu_stat_name[vpu_get_buffer_state(vbuf)]);
>> +
>> + to_vpu_stat_name(vpu_get_buffer_state(vbuf)));
>> if (seq_write(s, str, num))
>> return 0;
>> }
>> diff --git a/drivers/media/platform/amphion/vpu_msgs.c
>> b/drivers/media/platform/amphion/vpu_msgs.c
>> index f9eb488d1b5e..d0ead051f7d1 100644
>> --- a/drivers/media/platform/amphion/vpu_msgs.c
>> +++ b/drivers/media/platform/amphion/vpu_msgs.c
>> @@ -32,7 +32,7 @@ static void vpu_session_handle_start_done(struct
>> vpu_inst *inst, struct vpu_rpc_
>>
>> static void vpu_session_handle_mem_request(struct vpu_inst *inst,
>> struct vpu_rpc_event *pkt) {
>> - struct vpu_pkt_mem_req_data req_data;
>> + struct vpu_pkt_mem_req_data req_data = { 0 };
>>
>> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&req_data);
>> vpu_trace(inst->dev, "[%d] %d:%d %d:%d %d:%d\n", @@ -80,7 +80,7
>> @@ static void vpu_session_handle_resolution_change(struct vpu_inst
>> *inst, struct v
>>
>> static void vpu_session_handle_enc_frame_done(struct vpu_inst *inst,
>> struct vpu_rpc_event *pkt) {
>> - struct vpu_enc_pic_info info;
>> + struct vpu_enc_pic_info info = { 0 };
>>
>> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
>> dev_dbg(inst->dev, "[%d] frame id = %d, wptr = 0x%x, size =
>> %d\n", @@ -90,7 +90,7 @@ static void
>> vpu_session_handle_enc_frame_done(struct vpu_inst *inst, struct vpu_
>>
>> static void vpu_session_handle_frame_request(struct vpu_inst *inst,
>> struct vpu_rpc_event *pkt) {
>> - struct vpu_fs_info fs;
>> + struct vpu_fs_info fs = { 0 };
>>
>> vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
>> call_void_vop(inst, event_notify, VPU_MSG_ID_FRAME_REQ, &fs); @@
>> -107,7 +107,7 @@ static void vpu_session_handle_frame_release(struct
>vpu_inst *inst, struct vpu_r
>> info.type = inst->out_format.type;
>> call_void_vop(inst, buf_done, &info);
>> } else if (inst->core->type == VPU_CORE_TYPE_DEC) {
>> - struct vpu_fs_info fs;
>> + struct vpu_fs_info fs = { 0 };
>>
>> vpu_iface_unpack_msg_data(inst->core, pkt, &fs);
>> call_void_vop(inst, event_notify,
>> VPU_MSG_ID_FRAME_RELEASE, &fs); @@ -122,7 +122,7 @@ static void
>> vpu_session_handle_input_done(struct vpu_inst *inst, struct vpu_rpc_
>>
>> static void vpu_session_handle_pic_decoded(struct vpu_inst *inst,
>> struct vpu_rpc_event *pkt) {
>> - struct vpu_dec_pic_info info;
>> + struct vpu_dec_pic_info info = { 0 };
>>
>> vpu_iface_unpack_msg_data(inst->core, pkt, (void *)&info);
>> call_void_vop(inst, get_one_frame, &info); @@ -130,7 +130,7 @@
>> static void vpu_session_handle_pic_decoded(struct vpu_inst *inst,
>> struct vpu_rpc
>>
>> static void vpu_session_handle_pic_done(struct vpu_inst *inst, struct
>> vpu_rpc_event *pkt) {
>> - struct vpu_dec_pic_info info;
>> + struct vpu_dec_pic_info info = { 0 };
>> struct vpu_frame_info frame;
>>
>> memset(&frame, 0, sizeof(frame));
Le mardi 18 juillet 2023 à 01:50 +0000, Ming Qian a écrit :
> > > diff --git a/drivers/media/platform/amphion/venc.c
> > > b/drivers/media/platform/amphion/venc.c
> > > index 58480e2755ec..4eb57d793a9c 100644
> > > --- a/drivers/media/platform/amphion/venc.c
> > > +++ b/drivers/media/platform/amphion/venc.c
> > > @@ -268,7 +268,7 @@ static int venc_g_parm(struct file *file, void
> > > *fh, struct v4l2_streamparm *parm {
> > > struct vpu_inst *inst = to_inst(file);
> > > struct venc_t *venc = inst->priv;
> > > - struct v4l2_fract *timeperframe = &parm->parm.capture.timeperframe;
> > > + struct v4l2_fract *timeperframe;
> >
> > Could be just me, but I feel I'm missing some context to understand why this
> > change. Perhaps the commit message could be improved ?
> >
> > All other changes looks like improvement to me, so with a good explanation
> > on this one (and the change seems to be equivalent), you can add:
> >
> > Reviewed-by: Nicolas Dufresne <[email protected]>
> >
>
> Hi Nicolas,
> The Coverity scan report a REVERSE_INULL issue here, that directly dereferencing pointer "param", before Null-checking "parm".
> I'll split this patch into several patches, one topic one patch.
>
> Ming
Make sense now, looking forward a split version with more explanation.
regards,
Nicolas