Message ID | 1376636809-10159-7-git-send-email-tangchen@cn.fujitsu.com (mailing list archive) |
---|---|
State | RFC, archived |
Headers | show |
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
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 --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; } /*
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(-)