acpi_evaluate_object() has nesting if judgement to make code a little bit complicated, refactor it to make things simple.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/acpica/nsxfeval.c | 104 +++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/acpica/nsxfeval.c b/drivers/acpi/acpica/nsxfeval.c index 1f0c28b..a1b0b88 100644 --- a/drivers/acpi/acpica/nsxfeval.c +++ b/drivers/acpi/acpica/nsxfeval.c @@ -370,68 +370,64 @@ acpi_evaluate_object(acpi_handle handle, * If we are expecting a return value, and all went well above, * copy the return value to an external object. */ - if (return_buffer) { - if (!info->return_object) { - return_buffer->length = 0; - } else { - if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) == - ACPI_DESC_TYPE_NAMED) { - /* - * If we received a NS Node as a return object, this means that - * the object we are evaluating has nothing interesting to - * return (such as a mutex, etc.) We return an error because - * these types are essentially unsupported by this interface. - * We don't check up front because this makes it easier to add - * support for various types at a later date if necessary. - */ - status = AE_TYPE; - info->return_object = NULL; /* No need to delete a NS Node */ - return_buffer->length = 0; - } + if (!return_buffer) + goto out;
- if (ACPI_SUCCESS(status)) { + if (!info->return_object) { + return_buffer->length = 0; + goto out; + }
- /* Dereference Index and ref_of references */ + if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) == + ACPI_DESC_TYPE_NAMED) { + /* + * If we received a NS Node as a return object, this means that + * the object we are evaluating has nothing interesting to + * return (such as a mutex, etc.) We return an error because + * these types are essentially unsupported by this interface. + * We don't check up front because this makes it easier to add + * support for various types at a later date if necessary. + */ + status = AE_TYPE; + info->return_object = NULL; /* No need to delete a NS Node */ + return_buffer->length = 0; + }
- acpi_ns_resolve_references(info); + if (ACPI_FAILURE(status)) + goto out;
- /* Get the size of the returned object */ + /* Dereference Index and ref_of references */
- status = - acpi_ut_get_object_size(info->return_object, - &buffer_space_needed); - if (ACPI_SUCCESS(status)) { - - /* Validate/Allocate/Clear caller buffer */ - - status = - acpi_ut_initialize_buffer - (return_buffer, - buffer_space_needed); - if (ACPI_FAILURE(status)) { - /* - * Caller's buffer is too small or a new one can't - * be allocated - */ - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "Needed buffer size %X, %s\n", - (u32) - buffer_space_needed, - acpi_format_exception - (status))); - } else { - /* We have enough space for the object, build it */ - - status = - acpi_ut_copy_iobject_to_eobject - (info->return_object, - return_buffer); - } - } - } + acpi_ns_resolve_references(info); + + /* Get the size of the returned object */ + + status = acpi_ut_get_object_size(info->return_object, + &buffer_space_needed); + if (ACPI_SUCCESS(status)) { + + /* Validate/Allocate/Clear caller buffer */ + + status = acpi_ut_initialize_buffer(return_buffer, + buffer_space_needed); + if (ACPI_FAILURE(status)) { + /* + * Caller's buffer is too small or a new one can't + * be allocated + */ + ACPI_DEBUG_PRINT((ACPI_DB_INFO, + "Needed buffer size %X, %s\n", + (u32)buffer_space_needed, + acpi_format_exception(status))); + } else { + /* We have enough space for the object, build it */ + + status = acpi_ut_copy_iobject_to_eobject( + info->return_object, return_buffer); } }
+out: if (info->return_object) { /* * Delete the internal return object. NOTE: Interpreter must be
There is a potential memory leak when acpi_ut_copy_iobject_to_eobject() failed, because return_buffer was allocated in acpi_ut_initialize_buffer() when buffer type is ACPI_ALLOCATE_BUFFER or ACPI_ALLOCATE_LOCAL_BUFFER, and will not be freed outside when the return value is not AE_OK for acpi_evaluate_object(), fix it.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Yijing Wang wangyijing@huawei.com --- drivers/acpi/acpica/nsxfeval.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/acpica/nsxfeval.c b/drivers/acpi/acpica/nsxfeval.c index a1b0b88..8ad792b 100644 --- a/drivers/acpi/acpica/nsxfeval.c +++ b/drivers/acpi/acpica/nsxfeval.c @@ -424,6 +424,11 @@ acpi_evaluate_object(acpi_handle handle,
status = acpi_ut_copy_iobject_to_eobject( info->return_object, return_buffer); + if (ACPI_FAILURE(status) && + (buffer_space_needed == ACPI_ALLOCATE_BUFFER || + buffer_space_needed == ACPI_ALLOCATE_LOCAL_BUFFER)) { + ACPI_FREE(return_buffer); + } } }
Please file a bugzilla on this at:
thanks. Bob
-----Original Message----- From: Hanjun Guo [mailto:hanjun.guo@linaro.org] Sent: Friday, January 24, 2014 8:20 AM To: Zheng, Lv; Moore, Robert Cc: Rafael J. Wysocki; linux-acpi@vger.kernel.org; devel@acpica.org; patches@linaro.org; linux-kernel@vger.kernel.org; linaro- acpi@lists.linaro.org; Hanjun Guo Subject: [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if
acpi_evaluate_object() has nesting if judgement to make code a little bit complicated, refactor it to make things simple.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/acpica/nsxfeval.c | 104 +++++++++++++++++++----------------
1 file changed, 50 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/acpica/nsxfeval.c b/drivers/acpi/acpica/nsxfeval.c index 1f0c28b..a1b0b88 100644 --- a/drivers/acpi/acpica/nsxfeval.c +++ b/drivers/acpi/acpica/nsxfeval.c @@ -370,68 +370,64 @@ acpi_evaluate_object(acpi_handle handle, * If we are expecting a return value, and all went well above, * copy the return value to an external object. */
- if (return_buffer) {
if (!info->return_object) {return_buffer->length = 0;} else {if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==ACPI_DESC_TYPE_NAMED) {/** If we received a NS Node as a return object,this means that
* the object we are evaluating has nothinginteresting to
* return (such as a mutex, etc.) We return anerror because
* these types are essentially unsupported by thisinterface.
* We don't check up front because this makes iteasier to add
* support for various types at a later date ifnecessary.
*/status = AE_TYPE;info->return_object = NULL; /* No need to deletea NS Node */
return_buffer->length = 0;}
- if (!return_buffer)
goto out;
if (ACPI_SUCCESS(status)) {
- if (!info->return_object) {
return_buffer->length = 0;goto out;- }
/* Dereference Index and ref_of references */
- if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==
ACPI_DESC_TYPE_NAMED) {/** If we received a NS Node as a return object, this meansthat
* the object we are evaluating has nothing interesting to* return (such as a mutex, etc.) We return an error because* these types are essentially unsupported by this interface.* We don't check up front because this makes it easier to add* support for various types at a later date if necessary.*/status = AE_TYPE;info->return_object = NULL; /* No need to delete a NS Node*/
return_buffer->length = 0;- }
acpi_ns_resolve_references(info);
- if (ACPI_FAILURE(status))
goto out;
/* Get the size of the returned object */
- /* Dereference Index and ref_of references */
status =acpi_ut_get_object_size(info->return_object,&buffer_space_needed);if (ACPI_SUCCESS(status)) {/* Validate/Allocate/Clear caller buffer */status =acpi_ut_initialize_buffer(return_buffer,buffer_space_needed);if (ACPI_FAILURE(status)) {/** Caller's buffer is too small or anew one can't
* be allocated*/ACPI_DEBUG_PRINT((ACPI_DB_INFO,"Needed buffer size %X,%s\n",
(u32)buffer_space_needed,acpi_format_exception(status)));} else {/* We have enough space for theobject, build it */
status =acpi_ut_copy_iobject_to_eobject(info->return_object,return_buffer);}}}
- acpi_ns_resolve_references(info);
- /* Get the size of the returned object */
- status = acpi_ut_get_object_size(info->return_object,
&buffer_space_needed);- if (ACPI_SUCCESS(status)) {
/* Validate/Allocate/Clear caller buffer */status = acpi_ut_initialize_buffer(return_buffer,buffer_space_needed);if (ACPI_FAILURE(status)) {/** Caller's buffer is too small or a new one can't* be allocated*/ACPI_DEBUG_PRINT((ACPI_DB_INFO,"Needed buffer size %X, %s\n",(u32)buffer_space_needed,acpi_format_exception(status)));} else {/* We have enough space for the object, build it */status = acpi_ut_copy_iobject_to_eobject(info->return_object, return_buffer);+out: if (info->return_object) { /* * Delete the internal return object. NOTE: Interpreter must be -- 1.7.9.5
-
Hanjun Guo -
Moore, Robert