Re: [PATCH 1/2] make acpi_path_name() global (v2)

Randy Dunlap
2005-12-19T21:19:45Z

On Mon, 19 Dec 2005 14:19:25 -0800
Patrick Mochel <mochel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

> On Mon, Dec 19, 2005 at 11:54:35AM -0800, Randy Dunlap wrote:
> > From: Randy Dunlap <randy_d_dunlap-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >
> > Make acpi_path_name() usable by everyone.
> > I need this for adding SATA suspend/resume ACPI support.
>
> I liked the first one better, and I think it was more appropriate,
> given its users:
>
> It's a function that is used by code outside of drivers/acpi/, so it
> seems that it should use semantics that are common to the rest of the
> kernel, and not expose/require any internal data structures than what is
> necessary (like struct acpi_buffer).
>
> It seems like that is the point of the ACPI OSL - to provide functions
> that are stylistically and semantically familiar to the rest OS for
> which to interface with the ACPICA. In the case of Linux and this
> function, that would be more like providing an interface that you did
> before - returning a char * that the caller has to free.
>
> Note that Kristen made an argument that it's a burden on the caller,
> and a source for memory leak bugs. That's a valid concern, but there
> are many examples of similar interfaces (think strdup(3) or kstrdup()),
> and the effects can be mitigated by proper documentation. For the
> kernel, one could potentially integrate a check for that type of bad
> programming into sparse.. Besides, making the caller understand
> enough of the semantics of the ACPICA to initialize and tear down
> the acpi_buffer is arguably a much larger burden than knowing you
> have to free the string returned..

Thanks for enumerating.

These are all things that I considered in making the first
patch and most of the reasons that I chose to make that patch
as my first choice. I still consider it to be the more
appropriate patch than the second version.

---
~Randy