diff mbox

[6/6] acpi: Return -ENOENT in acpi_table_parse() and fix wrong comment.

Message ID 1376636809-10159-7-git-send-email-tangchen@cn.fujitsu.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

tangchen Aug. 16, 2013, 7:06 a.m. UTC
The comment about return value of acpi_table_parse() is incorrect.
This patch fix it.

Furthermore, if the table is not found, return 1 means nothing, and
make it difficult to write the comment. So return -ENOENT when the
table is not found, and correct the comment.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 drivers/acpi/tables.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Comments

Toshi Kani Aug. 19, 2013, 7:29 p.m. UTC | #1
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> The comment about return value of acpi_table_parse() is incorrect.
> This patch fix it.
> 
> Furthermore, if the table is not found, return 1 means nothing, and
> make it difficult to write the comment. So return -ENOENT when the
> table is not found, and correct the comment.

I am OK with the change, but the above description is not very clear.
You should state that all callers only check if the function succeeded
or not.  So, you are simplifying the semantics by returning -errno for
all failure cases.

Since you are making this change, I'd suggest you also update the stub
function in linux/acpi.h to return -ENODEV as well.

Thanks,
-Toshi

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
tangchen Aug. 20, 2013, 1:19 a.m. UTC | #2
On 08/20/2013 03:29 AM, Toshi Kani wrote:
> On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
>> The comment about return value of acpi_table_parse() is incorrect.
>> This patch fix it.
>>
>> Furthermore, if the table is not found, return 1 means nothing, and
>> make it difficult to write the comment. So return -ENOENT when the
>> table is not found, and correct the comment.
>
> I am OK with the change, but the above description is not very clear.
> You should state that all callers only check if the function succeeded
> or not.  So, you are simplifying the semantics by returning -errno for
> all failure cases.
>
> Since you are making this change, I'd suggest you also update the stub
> function in linux/acpi.h to return -ENODEV as well.

OK, followed. And will merge patch 3 and 4 and resend them later.

Thanks for reviewing.


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 5a5263b..e6de24f 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -278,12 +278,13 @@  acpi_table_parse_madt(enum acpi_madt_type id,
 
 /**
  * acpi_table_parse - find table with @id, run @handler on it
- *
  * @id: table id to find
  * @handler: handler to run
  *
  * Scan the ACPI System Descriptor Table (STD) for a table matching @id,
- * run @handler on it.  Return 0 if table found, return on if not.
+ * run @handler on it.
+ *
+ * Return 0 on success, -errno on failure.
  */
 int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
 {
@@ -306,7 +307,7 @@  int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
 		early_acpi_os_unmap_memory(table, tbl_size);
 		return 0;
 	} else
-		return 1;
+		return -ENOENT;
 }
 
 /*