Re: [PATCH v2 3/5] ACPI: Support __free() from cleanup.h for ACPI objects

On Tue, Aug 25, 2026 at 3:02 PM Rafael J. Wysocki (Intel)
<[email protected]> wrote:
>
> On Tue, Aug 25, 2026 at 2:32 PM Ilpo Järvinen
> <[email protected]> wrote:
> >
> > On Tue, 25 Aug 2026, Rafael J. Wysocki (Intel) wrote:
> >
> > > On Tue, Aug 25, 2026 at 2:07 PM Sakari Ailus
> > > <[email protected]> wrote:
> > > >
> > > > Hi Rafael,
> > > >
> > > > On Tue, Aug 25, 2026 at 01:51:18PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > > > On Mon, Aug 24, 2026 at 11:13 PM Sakari Ailus
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > Use DEFINE_FREE() to allow ACPI objects to be released automatically.
> > > > >
> > > > > But at least some of them are allocated by ACPICA functions like
> > > > > acpi_evaluate_object() and so they have no proper constructors.
> > > >
> > > > You could still assign the return buffer to a local variable. It's not
> > > > ideal API-wise though.
> > >
> > > Exactly.
> > >
> > > > I'm not quite sure what was the point you wanted to make but I reckon this
> > > > wasn't an ack. :-)
> > >
> > > Using the _FREE with variables that are not initialized through a
> > > constructor is questionable, so this is generally not particularly
> > > clean.
> >
> > The driver does call ACPI_FREE() for that pointer so clearly it's already
> > using something ending with "_FREE" already. So unless Rafael is
> > suggesting ACPI_FREE() should be renamed, I'm a bit lost what that
> > even means on concrete terms.
>
> Sorry for the confusion.
>
> I just don't want people to do things like this:
>
>     struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER };
>     union acpi_object *out_obj __free(ACPI_FREE);
>     acpi_status status;
>
>     status = acpi_evaluate_object(handle, METHOD_NAME, NULL, &output);
>     if (ACPI_FAILURE(status))
>         return AN_ERROR;
>
>    out_obj = output.pointer;
>
> > > There is no cleanup.h in ACPICA that is a more traditional C code
> > > base, so mixing up ACPICA code, which ACPI_FREE() is strictly
> > > speaking, with cleanup.h stuff is not particularly straightforward
> > > IMV.  I'd rather not do it.
> >
> > Perhaps add the DEFINE_FREE() into int3472 driver then, it seems to be
> > what intel/pmc is already doing (I don't seem to anymore recall why it was
> > added there). Using cleanup.h for that variable it clearly simplifies the
> > code flow.
>
> Well, fair enough, but as I said elsewhere, the code flow
> simplification can also be achieved in a different way.
>
> > It feels a bit stupid to duplicate it there but I guess we'll
> > just have to live with that if there's no place in any acpi related
> > headers for cleanup.h.
>
> If there is a cleanup.h "free" that can only be used with objects
> returned by acpi_evaluate_dsm_typed(), I'll be fine with that.
>
> Or if everyone agrees that doing
>
>     union acpi_object *out_obj __free(ACPI_FREE) = NULL;
>
> is not confusing and fine, I may just say "Hey, I don't care that much".

And particularly there is this paragraph in a comment in cleanup.h:

 * Given that the "__free(...) = NULL" pattern for variables defined at
 * the top of the function poses this potential interdependency problem
 * the recommendation is to always define and assign variables in one
 * statement and not group variable definitions at the top of the
 * function when __free() is used.

regarding a broken code example, so I would think that this is not a
made-up concern.