Hi Am 20.06.23 um 18:06 schrieb Sui Jingfeng:
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 returnNo indention please.OK, will be fixed at the next version.Signed-off-by: Sui Jingfeng <15330273260@xxxxxx> --- 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.
The ->vmap() callback can succeed with ret=0, but we still have no memory. Then we return -ENOMEM. The call to _is_null(map) is part of the error handling for ->vmap(). That is a bit strange, but it as always worked like that. Keeping all error handling in the same if-else block make all this more obvious.
Best regards Thomas
But both case (1. and 2.) are same in the semantic, right?Best regards Thomasreturn 0;
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
