[PATCH] drm/drm_gem.c: remove surplus else after return clause

Sui Jingfeng suijingfeng at loongson.cn
Tue Jun 20 16:06:48 UTC 2023


Hi,

On 2023/6/20 22:43, Thomas Zimmermann wrote:
> Hi
>
> Am 20.06.23 um 06:08 schrieb Sui Jingfeng:
>> ping ?
>>
>> On 2023/3/14 20:53, Sui Jingfeng wrote:
>>>   else is not generally useful after return
>
> No indention please.
>
OK, will be fixed at the next version.
>>>
>>> Signed-off-by: Sui Jingfeng <15330273260 at 189.cn>
>>> ---
>>>   drivers/gpu/drm/drm_gem.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>> index a6208e2c089b..364e3733af98 100644
>>> --- a/drivers/gpu/drm/drm_gem.c
>>> +++ b/drivers/gpu/drm/drm_gem.c
>>> @@ -1150,8 +1150,8 @@ int drm_gem_pin(struct drm_gem_object *obj)
>>>   {
>>>       if (obj->funcs->pin)
>>>           return obj->funcs->pin(obj);
>>> -    else
>>> -        return 0;
>>> +
>>> +    return 0;
>
> This change is ok.
>
>>>   }
>>>   void drm_gem_unpin(struct drm_gem_object *obj)
>>> @@ -1172,7 +1172,8 @@ int drm_gem_vmap(struct drm_gem_object *obj, 
>>> struct iosys_map *map)
>>>       ret = obj->funcs->vmap(obj, map);
>>>       if (ret)
>>>           return ret;
>>> -    else if (iosys_map_is_null(map))
>>> +
>>> +    if (iosys_map_is_null(map))
>>>           return -ENOMEM;
>
> This is not correct. Calling iosys_map_is_null() is part of handling 
> the return value from vmap, so the else is fine.
>
Are you serious ?


1. Before apply this patch:


If the 'ret' is 0,  it stand for obj->funcs->vmap() is successful, then 
if (iosys_map_is_null(map)) will be run.

If the 'ret' is NOT 0, then it return immediately.


2. After apply this patch:


If the 'ret' is NOT 0, it stand for obj->funcs->vmap() failed, then it 
return immediately.

If the 'ret' is 0, it stand for obj->funcs->vmap() is successful, then 
the check if (iosys_map_is_null(map))

will be run!


I feel strange about the core here, I think the check ' if 
(iosys_map_is_null(map))' is not needed,

the implement should responsible to handle all of possible errors.


But both case (1. and 2.) are same in the semantic, right?


> Best regards
> Thomas
>
>>>       return 0;
>>
>
-- 
Jingfeng



More information about the dri-devel mailing list