Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 2/11/2025 4:19 PM, Abhinav Kumar wrote:
> 
> 
> On 2/11/2025 4:13 PM, Dmitry Baryshkov wrote:
>> On Tue, Feb 11, 2025 at 10:23:54AM +0100, Marijn Suijten wrote:
>>> On 2025-02-10 14:14:14, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
>>>>> There is a possibility for an uninitialized *ret* variable to be
>>>>> returned in some code paths.
>>>>>
>>>>> Fix this by initializing *ret* to 0.
>>>>>
>>>>> Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
>>>>> Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
>>>>> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Return explicit 0 when no error occurs
>>>>> - Add hardening mailing lists
>>>>> - Link to v1: https://lore.kernel.org/r/20250209-dpu- 
>>>>> v1-1-0db666884f70@ethancedwards.com
>>>>> ---
>>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
>>>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>
>>>> Thanks for your patch, this was addressed with
>>>>
>>>> https://patchwork.freedesktop.org/patch/631567/ but since this is 
>>>> better
>>>> I am fine with this, will pick this one up
>>>
>>> The `return 0;` in this patch should certainly fix this issue 
>>> entirely and we
>>> don't need to inline the `int ret` for that, which I think is against 
>>> mixed
>>> declaration rules anyway?
>>>
>>> As far as I understand that's what Dmitry suggested in v1, but he r- 
>>> b'd it in
>>> this form.  Dmitry, was that intended?
>>
>> I think it should be fine, if the gcc doesn't warn against it.
>>
> 
> Let me test out the compilation while applying and see if it throws any 
> errors. If it does, will report here and we can go with the other patch.
> 

On my end, compilation looks fine, so will pickup this version.

Thanks

Abhinav
>>>
>>> - Marijn
>>>
>>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/ 
>>>>> gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> index 
>>>>> 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct 
>>>>> dpu_global_state *global_state,
>>>>>                       unsigned int num_planes)
>>>>>    {
>>>>>        unsigned int i;
>>>>> -    int ret;
>>>>>        for (i = 0; i < num_planes; i++) {
>>>>>            struct drm_plane_state *plane_state = states[i];
>>>>> @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct 
>>>>> dpu_global_state *global_state,
>>>>>                !plane_state->visible)
>>>>>                continue;
>>>>> -        ret = dpu_plane_virtual_assign_resources(crtc, global_state,
>>>>> +        int ret = dpu_plane_virtual_assign_resources(crtc, 
>>>>> global_state,
>>>>>                                 state, plane_state);
>>>>>            if (ret)
>>>>> -            break;
>>>>> +            return ret;
>>>>>        }
>>>>> -    return ret;
>>>>> +    return 0;
>>>>>    }
>>>>>    static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct 
>>>>> dpu_sw_pipe *pipe)
>>>>>
>>>>> ---
>>>>> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
>>>>> change-id: 20250209-dpu-c3fac78fc617
>>>>>
>>>>> Best regards,
>>>>
>>
>