Skip to contentSkip to conversation

make acpi_path_name() global (v2)

PATCH In review Randy Dunlap 3 participants 4 messages Dec 19, 2005 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 make acpi_path_name() global (v2)

From: Randy Dunlap <randy_d_dunlap-VuQAYsv1563Yd54FQh9/[email protected]>

Make acpi_path_name() usable by everyone.
I need this for adding SATA suspend/resume ACPI support.

---
 drivers/acpi/osl.c      |   21 +++++++++++++++++++++
 include/acpi/acpiosxf.h |    2 ++
 2 files changed, 23 insertions(+)
drivers/acpi/osl.c +21 -0
@@ -1078,6 +1078,27 @@ void acpi_os_release_lock(acpi_handle ha
 	spin_unlock_irqrestore((spinlock_t *) handle, flags);
 }
 
+/**
+ * acpi_path_name - get ACPI path_name for the given handle
+ * @handle: ACPI object handle to look up name of
+ * @namebuf: acpi_buffer with input flags and output name
+ *
+ * Caller can allocate & free the output name buffer or can set
+ * namebuf.length to ACPI_ALLOCATE_BUFFER to have ACPI-CA allocate
+ * the buffer but caller is still responsible for freeing it.
+ *
+ * Returns: status of acpi_get_name()
+ */
+acpi_status acpi_path_name(acpi_handle handle, struct acpi_buffer *namebuf)
+{
+	acpi_status		status;
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, namebuf);
+
+	return_ACPI_STATUS(status);
+}
+EXPORT_SYMBOL(acpi_path_name);
+
 #ifndef ACPI_USE_LOCAL_CACHE
 
 /*******************************************************************************
include/acpi/acpiosxf.h +2 -0
@@ -112,6 +112,8 @@ unsigned long acpi_os_acquire_lock(acpi_
 
 void acpi_os_release_lock(acpi_handle handle, unsigned long flags);
 
+acpi_status acpi_path_name(acpi_handle handle, struct acpi_buffer *namebuf);
+
 /*
  * Memory allocation and mapping
  */

---

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

A message that is not in the archive (sent off-list or not yet received): 20051219115435.13ea7bed.randy_d_dunlap-VuQAYsv1563Yd54FQh9/[email protected]

#2 lore raw
Quoted 28 lines · Randy Dunlap

From: Randy Dunlap <randy_d_dunlap-VuQAYsv1563Yd54FQh9/[email protected]>

Make acpi_path_name() usable by everyone.
I need this for adding SATA suspend/resume ACPI support.

---
 drivers/acpi/osl.c      |   21 +++++++++++++++++++++
 include/acpi/acpiosxf.h |    2 ++
 2 files changed, 23 insertions(+)
drivers/acpi/osl.c +11 -0
@@ -1078,6 +1078,27 @@ void acpi_os_release_lock(acpi_handle ha
 	spin_unlock_irqrestore((spinlock_t *) handle, flags);
 }
 
+/**
+ * acpi_path_name - get ACPI path_name for the given handle
+ * @handle: ACPI object handle to look up name of
+ * @namebuf: acpi_buffer with input flags and output name
+ *
+ * Caller can allocate & free the output name buffer or can set
+ * namebuf.length to ACPI_ALLOCATE_BUFFER to have ACPI-CA allocate
+ * the buffer but caller is still responsible for freeing it.
+ *
+ * Returns: status of acpi_get_name()
+ */

drivers/acpi/osl.c void acpi_os_release_lock(acpi_handle ha · line 1101

Quoted 10 lines
drivers/acpi/osl.c +10 -0
+acpi_status acpi_path_name(acpi_handle handle, struct acpi_buffer *namebuf)
+{
+	acpi_status		status;
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, namebuf);
+
+	return_ACPI_STATUS(status);
+}
+EXPORT_SYMBOL(acpi_path_name);
+

Shouldn't this be EXPORT_SYMBOL_GPL instead?

Kristen

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

#3 lore raw
In reply to a message not in the archive
Quoted 4 lines · Randy Dunlap

From: Randy Dunlap <randy_d_dunlap-VuQAYsv1563Yd54FQh9/[email protected]>

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,

Patrick

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

A message that is not in the archive (sent off-list or not yet received): 20051219221925.GA20943-VuQAYsv1563Yd54FQh9/[email protected]

#5 lore raw
Quoted 20 lines · On Mon, 19 Dec 2005 14:19:25 -0800 Patrick Mochel…
Quoted 4 lines · Randy Dunlap

From: Randy Dunlap <randy_d_dunlap-VuQAYsv1563Yd54FQh9/[email protected]>

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.

Quoted 9 lines

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

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click