linux-kernel - RE: [PATCH v2] ACPI / tables: Fix DSDT override mechanism

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1AE640813FDE7649BE1B193DEA596E883BB9B156@SHSMSX101.ccr.corp.intel.com>
Date:	Fri, 6 May 2016 00:31:01 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
CC:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] ACPI / tables: Fix DSDT override mechanism

Hi,

I also noticed this and was about to write a fix patch.
The build issue wasn't caught by LKP, I noticed it during ACPICA release test.
Thanks for doing this. :-)

> From: linux-acpi-owner@...r.kernel.org [mailto:linux-acpi-
> owner@...r.kernel.org] On Behalf Of Rafael J. Wysocki
> Subject: [PATCH v2] ACPI / tables: Fix DSDT override mechanism
> 
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> Commit 5ae74f2cc2f1 (ACPI / tables: Move table override mechanisms to
> tables.c) forgot to move the CONFIG_ACPI_CUSTOM_DSDT_FILE inclusion
> directive from osl.c to tables.c.  Fix that.
> 
> Fixes: 5ae74f2cc2f1 (ACPI / tables: Move table override mechanisms to tables.c)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> 
> A cosmetic change.
> 
> v1 -> v2: Move the new #include closer to the other #include directives in
>           tables.c.
> 
> ---
>  drivers/acpi/osl.c    |    4 ----
>  drivers/acpi/tables.c |    4 ++++
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/tables.c
> ================================================================
> ===
> --- linux-pm.orig/drivers/acpi/tables.c
> +++ linux-pm/drivers/acpi/tables.c
> @@ -36,6 +36,10 @@
>  #include <linux/memblock.h>
>  #include "internal.h"
> 
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT
> +#include CONFIG_ACPI_CUSTOM_DSDT_FILE
> +#endif
> +
[Lv Zheng] 
IMO, we could move this closer to acpi_os_table_override.
And move CONFIG_ACPI_CUSTOM_DSDT out of this function to form the following style:

#ifdef CONFIG_ACPI_CUSTOM_DSDT
#include CONFIG_ACPI_CUSTOM_DSDT_FILE

acpi_status acpi_os_table_override(...)
{
}
#else
acpi_status acpi_os_table_override(...)
{
	return AE_SUPPORT;
}
#endif

We can return AE_SUPPORT from acpi_os_table_override() here, please refer to drivers/acpi/acpica/tbinstal.c.
By doing so, we can:
1. make acpi_os_table_override clearer, and
2. make #ifdef maintained at function boundary, and
3. remove additional #ifdef CONFIG_ACPI_CUSTOM_DSDT, and
4. help to avoid same future mistakes.

Thanks and best regards
-Lv

>  #define ACPI_MAX_TABLES		128
> 
>  static char *mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
> Index: linux-pm/drivers/acpi/osl.c
> ================================================================
> ===
> --- linux-pm.orig/drivers/acpi/osl.c
> +++ linux-pm/drivers/acpi/osl.c
> @@ -56,10 +56,6 @@ struct acpi_os_dpc {
>  	struct work_struct work;
>  };
> 
> -#ifdef CONFIG_ACPI_CUSTOM_DSDT
> -#include CONFIG_ACPI_CUSTOM_DSDT_FILE
> -#endif
> -
>  #ifdef ENABLE_DEBUGGER
>  #include <linux/kdb.h>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html