[PATCH V8 43/43] drm/colorop: Add destroy functions for color pipeline
Simon Ser
contact at emersion.fr
Thu Apr 10 16:18:10 UTC 2025
On Tuesday, April 1st, 2025 at 04:42, Alex Hung <alex.hung at amd.com> wrote:
> On 3/29/25 09:48, Simon Ser wrote:
>
> > I would prefer these functions to be introduced together with the
> > patches adding functions to create objects and adding the new fields.
> > That way it's easier to check the symmetry and at no point in the
> > series there are memory leaks.
>
> The object creation and new fields are introduced in different patches.
> I divided this patch by introducing these functions in a patch, and 2.
> adding callers when needed to avoid memory leaks.
Could we introduce the destructor together with the function creating a
colorop, as a thin kfree() wrapper, and add cleanup for new fields in
that destructor in the same commit where the new fields are introduced?
> > Additionally, I would avoid using the name "cleanup", which seems to
> > have different semantics: for instance drm_plane_cleanup() doesn't kfree
> > the pointer. "destroy" seems more appropriate here.
>
> How about the following changes, i.e., freeing pointer is moved out of
> the cleanup function, and keeping the names.
I don't really see the upside, because (1) the creator function allocates
(so there is an asymmetry between the creator and destructor) and (2) all
callers of the destructor function will always want to kfree(), but I'd be
fine with that.
More information about the wayland-devel
mailing list