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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3619 bytes --]

On Tue, 25 Aug 2026, Rafael J. Wysocki (Intel) wrote:

> On Tue, Aug 25, 2026 at 2:32 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Tue, 25 Aug 2026, Rafael J. Wysocki (Intel) wrote:
> >
> > > On Tue, Aug 25, 2026 at 2:07 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> 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
> > > > > <sakari.ailus@linux.intel.com> 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.

So you'd be fine with something like this:

      union acpi_object *out_obj __free(some_other_name_than_ACPI_FREE) = ...

?

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

I personally don't hang myself into "free" or "constructor" terminology 
but look it more pragmatically, if ACPI_FREE() should be called before 
the object goes out of scope, __free() is a tool for that.

Have Rust people tried to wrap this interface already (there seems to 
acpi.rs but it seems very limited in scope)? I'd expect them to be just as 
interested in knowing what to do when something is at the end of its
lifetime.

-- 
 i.