On Thu, 17 Nov 2022 15:56:09 +0800, Simon Ser wrote: >> @@ -638,11 +638,14 @@ static int get_plane_modifiers(struct amdgpu_device *adev, unsigned int plane_ty >> return 0; >> >> *mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL); >> + if (!*mods) >> + return -ENOMEM; >> + >> >> if (plane_type == DRM_PLANE_TYPE_CURSOR) { >> add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_LINEAR); >> add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_INVALID); >> - return *mods ? 0 : -ENOMEM; >> + return 0; >> } >> >> switch (adev->family) { >> @@ -671,9 +674,6 @@ static int get_plane_modifiers(struct amdgpu_device *adev, unsigned int plane_ty >> /* INVALID marks the end of the list. */ >> add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_INVALID); >> >> - if (!*mods) >> - return -ENOMEM; >> - >> return 0; >> } > This breaks the "size" out-parameter. No, it will not change the value of the "size". The "size" can only be modified by add_modifier(). However, when the "*mods" is NULL, add_modifier() will return immediately, without the execution of "*size += 1;". Therefore, when the "*mods" is NULL, the rest of the function is useless, which should be better to skip. Jiang