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

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".