linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] acpi: set return value to const char for some functions
@ 2015-10-14 19:07 LABBE Corentin
  2015-10-14 19:48 ` kbuild test robot
  2015-10-14 20:53 ` Moore, Robert
  0 siblings, 2 replies; 12+ messages in thread
From: LABBE Corentin @ 2015-10-14 19:07 UTC (permalink / raw)
  To: robert.moore, lv.zheng, rafael.j.wysocki, lenb
  Cc: linux-acpi, devel, linux-kernel, LABBE Corentin

This patch set some array of const char as const.
In the same time, some function return pointer to thoses array without
properly giving the information that the data is const.
This patch set the return type of thoses functions as const char *

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/acpi/acpica/acutils.h  | 12 ++++++------
 drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
 drivers/acpi/acpica/uthex.c    |  4 ++--
 drivers/acpi/tables.c          |  6 ++++--
 4 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
index fb2aa50..e5d9d4f 100644
--- a/drivers/acpi/acpica/acutils.h
+++ b/drivers/acpi/acpica/acutils.h
@@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32 mutex_id);
 const char *acpi_ut_get_notify_name(u32 notify_value, acpi_object_type type);
 #endif
 
-char *acpi_ut_get_type_name(acpi_object_type type);
+const char *acpi_ut_get_type_name(acpi_object_type type);
 
 char *acpi_ut_get_node_name(void *object);
 
-char *acpi_ut_get_descriptor_name(void *object);
+const char *acpi_ut_get_descriptor_name(void *object);
 
 const char *acpi_ut_get_reference_name(union acpi_operand_object *object);
 
-char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc);
+const char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc);
 
-char *acpi_ut_get_region_name(u8 space_id);
+const char *acpi_ut_get_region_name(u8 space_id);
 
-char *acpi_ut_get_event_name(u32 event_id);
+const char *acpi_ut_get_event_name(u32 event_id);
 
-char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
+const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
 
 u8 acpi_ut_ascii_char_to_hex(int hex_char);
 
diff --git a/drivers/acpi/acpica/utdecode.c b/drivers/acpi/acpica/utdecode.c
index 988e23b..e08cdb1 100644
--- a/drivers/acpi/acpica/utdecode.c
+++ b/drivers/acpi/acpica/utdecode.c
@@ -114,7 +114,7 @@ const char *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
 	"PCC"			/* 0x0A */
 };
 
-char *acpi_ut_get_region_name(u8 space_id)
+const char *acpi_ut_get_region_name(u8 space_id)
 {
 
 	if (space_id >= ACPI_USER_REGION_BEGIN) {
@@ -127,7 +127,7 @@ char *acpi_ut_get_region_name(u8 space_id)
 		return ("InvalidSpaceId");
 	}
 
-	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
+	return (ACPI_CAST_PTR(const char, acpi_gbl_region_types[space_id]));
 }
 
 /*******************************************************************************
@@ -152,14 +152,14 @@ static const char *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
 	"RealTimeClock",
 };
 
-char *acpi_ut_get_event_name(u32 event_id)
+const char *acpi_ut_get_event_name(u32 event_id)
 {
 
 	if (event_id > ACPI_EVENT_MAX) {
 		return ("InvalidEventID");
 	}
 
-	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
+	return (ACPI_CAST_PTR(const char, acpi_gbl_event_types[event_id]));
 }
 
 /*******************************************************************************
@@ -220,17 +220,17 @@ static const char *acpi_gbl_ns_type_names[] = {
 	/* 30 */ "Invalid"
 };
 
-char *acpi_ut_get_type_name(acpi_object_type type)
+const char *acpi_ut_get_type_name(acpi_object_type type)
 {
 
 	if (type > ACPI_TYPE_INVALID) {
-		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
+		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
 	}
 
-	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
+	return (ACPI_CAST_PTR(const char, acpi_gbl_ns_type_names[type]));
 }
 
-char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc)
+const char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc)
 {
 
 	if (!obj_desc) {
@@ -318,7 +318,7 @@ static const char *acpi_gbl_desc_type_names[] = {
 	/* 15 */ "Node"
 };
 
-char *acpi_ut_get_descriptor_name(void *object)
+const char *acpi_ut_get_descriptor_name(void *object)
 {
 
 	if (!object) {
@@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void *object)
 		return ("Not a Descriptor");
 	}
 
-	return (ACPI_CAST_PTR(char,
+	return (ACPI_CAST_PTR(const char,
 			      acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
 						       (object)]));
 
@@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union acpi_operand_object *object)
 
 /* Names for internal mutex objects, used for debug output */
 
-static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
+static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
 	"ACPI_MTX_Interpreter",
 	"ACPI_MTX_Namespace",
 	"ACPI_MTX_Tables",
@@ -411,7 +411,7 @@ static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
 	"ACPI_MTX_CommandReady"
 };
 
-char *acpi_ut_get_mutex_name(u32 mutex_id)
+const char *acpi_ut_get_mutex_name(u32 mutex_id)
 {
 
 	if (mutex_id > ACPI_MAX_MUTEX) {
diff --git a/drivers/acpi/acpica/uthex.c b/drivers/acpi/acpica/uthex.c
index fda8b3d..9239711 100644
--- a/drivers/acpi/acpica/uthex.c
+++ b/drivers/acpi/acpica/uthex.c
@@ -48,7 +48,7 @@
 ACPI_MODULE_NAME("uthex")
 
 /* Hex to ASCII conversion table */
-static char acpi_gbl_hex_to_ascii[] = {
+static const char acpi_gbl_hex_to_ascii[] = {
 	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D',
 	    'E', 'F'
 };
@@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
  *
  ******************************************************************************/
 
-char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
+const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
 {
 
 	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]);
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 17a6fa0..7c8a037 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -35,8 +35,10 @@
 
 #define ACPI_MAX_TABLES		128
 
-static char *mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
-static char *mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level" };
+static const char * const mps_inti_flags_polarity[] = {
+	"dfl", "high", "res", "low" };
+static const char * const mps_inti_flags_trigger[] = {
+	"dfl", "edge", "res", "level" };
 
 static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES] __initdata;
 
-- 
2.4.9

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] acpi: set return value to const char for some functions
  2015-10-14 19:07 [PATCH] acpi: set return value to const char for some functions LABBE Corentin
@ 2015-10-14 19:48 ` kbuild test robot
  2015-10-14 20:53 ` Moore, Robert
  1 sibling, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2015-10-14 19:48 UTC (permalink / raw)
  Cc: kbuild-all, robert.moore, lv.zheng, rafael.j.wysocki, lenb,
	linux-acpi, devel, linux-kernel, LABBE Corentin

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

Hi LABBE,

[auto build test ERROR on pm/linux-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/LABBE-Corentin/acpi-set-return-value-to-const-char-for-some-functions/20151015-030935
config: i386-randconfig-s1-201541 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/acpi/acpica/utdecode.c:414:13: error: conflicting types for 'acpi_ut_get_mutex_name'
    const char *acpi_ut_get_mutex_name(u32 mutex_id)
                ^
   In file included from drivers/acpi/acpica/accommon.h:61:0,
                    from drivers/acpi/acpica/utdecode.c:45:
   drivers/acpi/acpica/acutils.h:187:7: note: previous declaration of 'acpi_ut_get_mutex_name' was here
    char *acpi_ut_get_mutex_name(u32 mutex_id);
          ^

vim +/acpi_ut_get_mutex_name +414 drivers/acpi/acpica/utdecode.c

   408		"ACPI_MTX_Caches",
   409		"ACPI_MTX_Memory",
   410		"ACPI_MTX_CommandComplete",
   411		"ACPI_MTX_CommandReady"
   412	};
   413	
 > 414	const char *acpi_ut_get_mutex_name(u32 mutex_id)
   415	{
   416	
   417		if (mutex_id > ACPI_MAX_MUTEX) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24803 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] acpi: set return value to const char for some functions
  2015-10-14 19:07 [PATCH] acpi: set return value to const char for some functions LABBE Corentin
  2015-10-14 19:48 ` kbuild test robot
@ 2015-10-14 20:53 ` Moore, Robert
  2015-10-15  2:14   ` Joe Perches
  2015-10-19 12:02   ` LABBE Corentin
  1 sibling, 2 replies; 12+ messages in thread
From: Moore, Robert @ 2015-10-14 20:53 UTC (permalink / raw)
  To: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org
  Cc: linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org

In ACPICA, we tend to be very careful concerning the "const" keyword in order to avoid a phenomenon known as "const pollution".

That is not to say that we won't use const in some limited cases.

Bob


> -----Original Message-----
> From: LABBE Corentin [mailto:clabbe.montjoie@gmail.com]
> Sent: Wednesday, October 14, 2015 12:07 PM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org
> Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> kernel@vger.kernel.org; LABBE Corentin
> Subject: [PATCH] acpi: set return value to const char for some functions
> 
> This patch set some array of const char as const.
> In the same time, some function return pointer to thoses array without
> properly giving the information that the data is const.
> This patch set the return type of thoses functions as const char *
> 
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/acpi/acpica/acutils.h  | 12 ++++++------
> drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
>  drivers/acpi/acpica/uthex.c    |  4 ++--
>  drivers/acpi/tables.c          |  6 ++++--
>  4 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
> index fb2aa50..e5d9d4f 100644
> --- a/drivers/acpi/acpica/acutils.h
> +++ b/drivers/acpi/acpica/acutils.h
> @@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32 mutex_id);  const
> char *acpi_ut_get_notify_name(u32 notify_value, acpi_object_type type);
> #endif
> 
> -char *acpi_ut_get_type_name(acpi_object_type type);
> +const char *acpi_ut_get_type_name(acpi_object_type type);
> 
>  char *acpi_ut_get_node_name(void *object);
> 
> -char *acpi_ut_get_descriptor_name(void *object);
> +const char *acpi_ut_get_descriptor_name(void *object);
> 
>  const char *acpi_ut_get_reference_name(union acpi_operand_object
> *object);
> 
> -char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc);
> +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> +*obj_desc);
> 
> -char *acpi_ut_get_region_name(u8 space_id);
> +const char *acpi_ut_get_region_name(u8 space_id);
> 
> -char *acpi_ut_get_event_name(u32 event_id);
> +const char *acpi_ut_get_event_name(u32 event_id);
> 
> -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> 
>  u8 acpi_ut_ascii_char_to_hex(int hex_char);
> 
> diff --git a/drivers/acpi/acpica/utdecode.c
> b/drivers/acpi/acpica/utdecode.c index 988e23b..e08cdb1 100644
> --- a/drivers/acpi/acpica/utdecode.c
> +++ b/drivers/acpi/acpica/utdecode.c
> @@ -114,7 +114,7 @@ const char
> *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
>  	"PCC"			/* 0x0A */
>  };
> 
> -char *acpi_ut_get_region_name(u8 space_id)
> +const char *acpi_ut_get_region_name(u8 space_id)
>  {
> 
>  	if (space_id >= ACPI_USER_REGION_BEGIN) { @@ -127,7 +127,7 @@ char
> *acpi_ut_get_region_name(u8 space_id)
>  		return ("InvalidSpaceId");
>  	}
> 
> -	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
> +	return (ACPI_CAST_PTR(const char, acpi_gbl_region_types[space_id]));
>  }
> 
> 
> /*************************************************************************
> ******
> @@ -152,14 +152,14 @@ static const char
> *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
>  	"RealTimeClock",
>  };
> 
> -char *acpi_ut_get_event_name(u32 event_id)
> +const char *acpi_ut_get_event_name(u32 event_id)
>  {
> 
>  	if (event_id > ACPI_EVENT_MAX) {
>  		return ("InvalidEventID");
>  	}
> 
> -	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
> +	return (ACPI_CAST_PTR(const char, acpi_gbl_event_types[event_id]));
>  }
> 
> 
> /*************************************************************************
> ******
> @@ -220,17 +220,17 @@ static const char *acpi_gbl_ns_type_names[] = {
>  	/* 30 */ "Invalid"
>  };
> 
> -char *acpi_ut_get_type_name(acpi_object_type type)
> +const char *acpi_ut_get_type_name(acpi_object_type type)
>  {
> 
>  	if (type > ACPI_TYPE_INVALID) {
> -		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
> +		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
>  	}
> 
> -	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
> +	return (ACPI_CAST_PTR(const char, acpi_gbl_ns_type_names[type]));
>  }
> 
> -char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc)
> +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> +*obj_desc)
>  {
> 
>  	if (!obj_desc) {
> @@ -318,7 +318,7 @@ static const char *acpi_gbl_desc_type_names[] = {
>  	/* 15 */ "Node"
>  };
> 
> -char *acpi_ut_get_descriptor_name(void *object)
> +const char *acpi_ut_get_descriptor_name(void *object)
>  {
> 
>  	if (!object) {
> @@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void *object)
>  		return ("Not a Descriptor");
>  	}
> 
> -	return (ACPI_CAST_PTR(char,
> +	return (ACPI_CAST_PTR(const char,
>  			      acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
>  						       (object)]));
> 
> @@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union
> acpi_operand_object *object)
> 
>  /* Names for internal mutex objects, used for debug output */
> 
> -static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> +static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
>  	"ACPI_MTX_Interpreter",
>  	"ACPI_MTX_Namespace",
>  	"ACPI_MTX_Tables",
> @@ -411,7 +411,7 @@ static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
>  	"ACPI_MTX_CommandReady"
>  };
> 
> -char *acpi_ut_get_mutex_name(u32 mutex_id)
> +const char *acpi_ut_get_mutex_name(u32 mutex_id)
>  {
> 
>  	if (mutex_id > ACPI_MAX_MUTEX) {
> diff --git a/drivers/acpi/acpica/uthex.c b/drivers/acpi/acpica/uthex.c
> index fda8b3d..9239711 100644
> --- a/drivers/acpi/acpica/uthex.c
> +++ b/drivers/acpi/acpica/uthex.c
> @@ -48,7 +48,7 @@
>  ACPI_MODULE_NAME("uthex")
> 
>  /* Hex to ASCII conversion table */
> -static char acpi_gbl_hex_to_ascii[] = {
> +static const char acpi_gbl_hex_to_ascii[] = {
>  	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
> 'D',
>  	    'E', 'F'
>  };
> @@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
>   *
> 
> **************************************************************************
> ****/
> 
> -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
>  {
> 
>  	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]); diff --
> git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 17a6fa0..7c8a037
> 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -35,8 +35,10 @@
> 
>  #define ACPI_MAX_TABLES		128
> 
> -static char *mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
> -static char *mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level"
> };
> +static const char * const mps_inti_flags_polarity[] = {
> +	"dfl", "high", "res", "low" };
> +static const char * const mps_inti_flags_trigger[] = {
> +	"dfl", "edge", "res", "level" };
> 
>  static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES] __initdata;
> 
> --
> 2.4.9

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] acpi: set return value to const char for some functions
  2015-10-14 20:53 ` Moore, Robert
@ 2015-10-15  2:14   ` Joe Perches
  2015-10-15 19:32     ` Moore, Robert
  2015-10-19 12:02   ` LABBE Corentin
  1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2015-10-15  2:14 UTC (permalink / raw)
  To: Moore, Robert
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org

On Wed, 2015-10-14 at 20:53 +0000, Moore, Robert wrote:
> In ACPICA, we tend to be very careful concerning the "const" keyword in order to avoid a phenomenon known as "const pollution".
> 
> That is not to say that we won't use const in some limited cases.

Please describe the effects of "const pollution".

Why isn't it useful to update the functions that
don't modify function pointer arguments to const?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] acpi: set return value to const char for some functions
  2015-10-15  2:14   ` Joe Perches
@ 2015-10-15 19:32     ` Moore, Robert
  2015-10-15 23:59       ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Moore, Robert @ 2015-10-15 19:32 UTC (permalink / raw)
  To: Joe Perches
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org, Box, David E

> Please describe the effects of "const pollution".
> 
> Why isn't it useful to update the functions that don't modify function
> pointer arguments to const?

It's not that const isn't useful, but it can create problems, especially in existing code. It can bubble up to higher functions, causing lots of changes to existing variables and the definition of existing functions. Not to mention lots of casting and recasting.

Example quote:

"if you started to use "const" for some methods you usually forced to use this in most of your code. But the time spent for maintaining (typing, recompiling when some const is missing, etc.) of const-correctness in code seems greater than for fixing of possible (very rare) problems caused by not using of const-correctness at all."

Also, ACPICA has to be additionally careful because it must compile with many different C compilers.



All that being said, I went ahead and made the actual ACPICA changes corresponding to your patches. There were no pollution issues, and it actually simplified the code by eliminating the need for some uses of ACPI_CAST_PTR.

The only issue I found was here, so we won't do this one:

const char
AcpiUtHexToAsciiChar

\acpica\source\include\acutils.h(322) :
    warning C4180: qualifier applied to function type has no meaning; ignored


Here is the current patch to the actual ACPICA code (which is in a different format than the Linux version of the code):

diff --git a/source/components/namespace/nsxfname.c b/source/components/namespace/nsxfname.c
index 51cc4f4..c368c1f 100644
--- a/source/components/namespace/nsxfname.c
+++ b/source/components/namespace/nsxfname.c
@@ -249,7 +249,7 @@ AcpiGetName (
 {
     ACPI_STATUS             Status;
     ACPI_NAMESPACE_NODE     *Node;
-    char                    *NodeName;
+    const char              *NodeName;
 
 
     /* Parameter validation */
diff --git a/source/components/utilities/utdecode.c b/source/components/utilities/utdecode.c
index 580f891..3d927f5 100644
--- a/source/components/utilities/utdecode.c
+++ b/source/components/utilities/utdecode.c
@@ -191,7 +191,7 @@ const char        *AcpiGbl_RegionTypes[ACPI_NUM_PREDEFINED_REGIONS] =
 };
 
 
-char *
+const char *
 AcpiUtGetRegionName (
     UINT8                   SpaceId)
 {
@@ -213,7 +213,7 @@ AcpiUtGetRegionName (
         return ("InvalidSpaceId");
     }
 
-    return (ACPI_CAST_PTR (char, AcpiGbl_RegionTypes[SpaceId]));
+    return (AcpiGbl_RegionTypes[SpaceId]);
 }
 
 
@@ -241,7 +241,7 @@ static const char        *AcpiGbl_EventTypes[ACPI_NUM_FIXED_EVENTS] =
 };
 
 
-char *
+const char *
 AcpiUtGetEventName (
     UINT32                  EventId)
 {
@@ -251,7 +251,7 @@ AcpiUtGetEventName (
         return ("InvalidEventID");
     }
 
-    return (ACPI_CAST_PTR (char, AcpiGbl_EventTypes[EventId]));
+    return (AcpiGbl_EventTypes[EventId]);
 }
 
 
@@ -316,21 +316,21 @@ static const char           *AcpiGbl_NsTypeNames[] =
 };
 
 
-char *
+const char *
 AcpiUtGetTypeName (
     ACPI_OBJECT_TYPE        Type)
 {
 
     if (Type > ACPI_TYPE_INVALID)
     {
-        return (ACPI_CAST_PTR (char, AcpiGbl_BadType));
+        return (AcpiGbl_BadType);
     }
 
-    return (ACPI_CAST_PTR (char, AcpiGbl_NsTypeNames[Type]));
+    return (AcpiGbl_NsTypeNames[Type]);
 }
 
 
-char *
+const char *
 AcpiUtGetObjectTypeName (
     ACPI_OPERAND_OBJECT     *ObjDesc)
 {
@@ -372,7 +372,7 @@ AcpiUtGetObjectTypeName (
  *
  ******************************************************************************/
 
-char *
+const char *
 AcpiUtGetNodeName (
     void                    *Object)
 {
@@ -448,7 +448,7 @@ static const char           *AcpiGbl_DescTypeNames[] =
 };
 
 
-char *
+const char *
 AcpiUtGetDescriptorName (
     void                    *Object)
 {
@@ -463,9 +463,7 @@ AcpiUtGetDescriptorName (
         return ("Not a Descriptor");
     }
 
-    return (ACPI_CAST_PTR (char,
-        AcpiGbl_DescTypeNames[ACPI_GET_DESCRIPTOR_TYPE (Object)]));
-
+    return (AcpiGbl_DescTypeNames[ACPI_GET_DESCRIPTOR_TYPE (Object)]);
 }
 
 
@@ -542,7 +540,7 @@ AcpiUtGetReferenceName (
 
 /* Names for internal mutex objects, used for debug output */
 
-static char                 *AcpiGbl_MutexNames[ACPI_NUM_MUTEX] =
+static const char           *AcpiGbl_MutexNames[ACPI_NUM_MUTEX] =
 {
     "ACPI_MTX_Interpreter",
     "ACPI_MTX_Namespace",
@@ -552,7 +550,7 @@ static char                 *AcpiGbl_MutexNames[ACPI_NUM_MUTEX] =
     "ACPI_MTX_Memory",
 };
 
-char *
+const char *
 AcpiUtGetMutexName (
     UINT32                  MutexId)
 {
diff --git a/source/components/utilities/uthex.c b/source/components/utilities/uthex.c
index c652f6a..3a32003 100644
--- a/source/components/utilities/uthex.c
+++ b/source/components/utilities/uthex.c
@@ -122,7 +122,7 @@
 
 /* Hex to ASCII conversion table */
 
-static char                 AcpiGbl_HexToAscii[] =
+static const char           AcpiGbl_HexToAscii[] =
 {
     '0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'
 };
diff --git a/source/include/acutils.h b/source/include/acutils.h
index 4fc44ff..f9372a2 100644
--- a/source/include/acutils.h
+++ b/source/include/acutils.h
@@ -278,7 +278,7 @@ AcpiUtInitGlobals (
 
 #if defined(ACPI_DEBUG_OUTPUT) || defined(ACPI_DEBUGGER)
 
-char *
+const char *
 AcpiUtGetMutexName (
     UINT32                  MutexId);
 
@@ -288,15 +288,15 @@ AcpiUtGetNotifyName (
     ACPI_OBJECT_TYPE        Type);
 #endif
 
-char *
+const char *
 AcpiUtGetTypeName (
     ACPI_OBJECT_TYPE        Type);
 
-char *
+const char *
 AcpiUtGetNodeName (
     void                    *Object);
 
-char *
+const char *
 AcpiUtGetDescriptorName (
     void                    *Object);
 
@@ -304,15 +304,15 @@ const char *
 AcpiUtGetReferenceName (
     ACPI_OPERAND_OBJECT     *Object);
 
-char *
+const char *
 AcpiUtGetObjectTypeName (
     ACPI_OPERAND_OBJECT     *ObjDesc);
 
-char *
+const char *
 AcpiUtGetRegionName (
     UINT8                   SpaceId);
 
-char *
+const char *
 AcpiUtGetEventName (
     UINT32                  EventId);


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] acpi: set return value to const char for some functions
  2015-10-15 19:32     ` Moore, Robert
@ 2015-10-15 23:59       ` Joe Perches
  2015-10-16  3:37         ` Moore, Robert
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2015-10-15 23:59 UTC (permalink / raw)
  To: Moore, Robert
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org, Box, David E

On Thu, 2015-10-15 at 19:32 +0000, Moore, Robert wrote:
> if you started to use "const" for some methods you usually forced to
> use this in most of your code. But the time spent for maintaining
> (typing, recompiling when some const is missing, etc.) of
> const-correctness in code seems greater than for fixing of possible
> (very rare) problems caused by not using of const-correctness at all

c is not c++.

"seems" is a dubious statement.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] acpi: set return value to const char for some functions
  2015-10-15 23:59       ` Joe Perches
@ 2015-10-16  3:37         ` Moore, Robert
  2015-10-16  3:47           ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Moore, Robert @ 2015-10-16  3:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org, Box, David E

If you don't like the quote, just stick with my first assessment.


> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Thursday, October 15, 2015 5:00 PM
> To: Moore, Robert
> Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org; linux-
> acpi@vger.kernel.org; devel@acpica.org; linux-kernel@vger.kernel.org; Box,
> David E
> Subject: Re: [PATCH] acpi: set return value to const char for some
> functions
> 
> On Thu, 2015-10-15 at 19:32 +0000, Moore, Robert wrote:
> > if you started to use "const" for some methods you usually forced to
> > use this in most of your code. But the time spent for maintaining
> > (typing, recompiling when some const is missing, etc.) of
> > const-correctness in code seems greater than for fixing of possible
> > (very rare) problems caused by not using of const-correctness at all
> 
> c is not c++.
> 
> "seems" is a dubious statement.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] acpi: set return value to const char for some functions
  2015-10-16  3:37         ` Moore, Robert
@ 2015-10-16  3:47           ` Joe Perches
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2015-10-16  3:47 UTC (permalink / raw)
  To: Moore, Robert
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org, Box, David E

On Fri, 2015-10-16 at 03:37 +0000, Moore, Robert wrote:
> If you don't like the quote, just stick with my first assessment.

Thanks, but if you can't make arguments yourself, it seems
you're making assertions rather than assessments.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] acpi: set return value to const char for some functions
  2015-10-14 20:53 ` Moore, Robert
  2015-10-15  2:14   ` Joe Perches
@ 2015-10-19 12:02   ` LABBE Corentin
  2015-10-19 17:04     ` Moore, Robert
  2015-10-19 17:15     ` Moore, Robert
  1 sibling, 2 replies; 12+ messages in thread
From: LABBE Corentin @ 2015-10-19 12:02 UTC (permalink / raw)
  To: Moore, Robert
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org

On Wed, Oct 14, 2015 at 08:53:19PM +0000, Moore, Robert wrote:
> In ACPICA, we tend to be very careful concerning the "const" keyword in order to avoid a phenomenon known as "const pollution".
> 
> That is not to say that we won't use const in some limited cases.
> 
> Bob
> 

Hello

The problem is that all function in this patch return a pointer to string literal without giving that information.
I am sad to see that you seems to agree with that but, for resuming your conversation with Joe Perches, you reject this patch just because you worry that perhaps in the future it will make you more works do deal with it.

Does you will accept the subset of this patch for adding const to "static char xxx[] = {}" ?

Regards

> 
> > -----Original Message-----
> > From: LABBE Corentin [mailto:clabbe.montjoie@gmail.com]
> > Sent: Wednesday, October 14, 2015 12:07 PM
> > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org
> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > kernel@vger.kernel.org; LABBE Corentin
> > Subject: [PATCH] acpi: set return value to const char for some functions
> > 
> > This patch set some array of const char as const.
> > In the same time, some function return pointer to thoses array without
> > properly giving the information that the data is const.
> > This patch set the return type of thoses functions as const char *
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > ---
> >  drivers/acpi/acpica/acutils.h  | 12 ++++++------
> > drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
> >  drivers/acpi/acpica/uthex.c    |  4 ++--
> >  drivers/acpi/tables.c          |  6 ++++--
> >  4 files changed, 24 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
> > index fb2aa50..e5d9d4f 100644
> > --- a/drivers/acpi/acpica/acutils.h
> > +++ b/drivers/acpi/acpica/acutils.h
> > @@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32 mutex_id);  const
> > char *acpi_ut_get_notify_name(u32 notify_value, acpi_object_type type);
> > #endif
> > 
> > -char *acpi_ut_get_type_name(acpi_object_type type);
> > +const char *acpi_ut_get_type_name(acpi_object_type type);
> > 
> >  char *acpi_ut_get_node_name(void *object);
> > 
> > -char *acpi_ut_get_descriptor_name(void *object);
> > +const char *acpi_ut_get_descriptor_name(void *object);
> > 
> >  const char *acpi_ut_get_reference_name(union acpi_operand_object
> > *object);
> > 
> > -char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc);
> > +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> > +*obj_desc);
> > 
> > -char *acpi_ut_get_region_name(u8 space_id);
> > +const char *acpi_ut_get_region_name(u8 space_id);
> > 
> > -char *acpi_ut_get_event_name(u32 event_id);
> > +const char *acpi_ut_get_event_name(u32 event_id);
> > 
> > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > 
> >  u8 acpi_ut_ascii_char_to_hex(int hex_char);
> > 
> > diff --git a/drivers/acpi/acpica/utdecode.c
> > b/drivers/acpi/acpica/utdecode.c index 988e23b..e08cdb1 100644
> > --- a/drivers/acpi/acpica/utdecode.c
> > +++ b/drivers/acpi/acpica/utdecode.c
> > @@ -114,7 +114,7 @@ const char
> > *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
> >  	"PCC"			/* 0x0A */
> >  };
> > 
> > -char *acpi_ut_get_region_name(u8 space_id)
> > +const char *acpi_ut_get_region_name(u8 space_id)
> >  {
> > 
> >  	if (space_id >= ACPI_USER_REGION_BEGIN) { @@ -127,7 +127,7 @@ char
> > *acpi_ut_get_region_name(u8 space_id)
> >  		return ("InvalidSpaceId");
> >  	}
> > 
> > -	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
> > +	return (ACPI_CAST_PTR(const char, acpi_gbl_region_types[space_id]));
> >  }
> > 
> > 
> > /*************************************************************************
> > ******
> > @@ -152,14 +152,14 @@ static const char
> > *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
> >  	"RealTimeClock",
> >  };
> > 
> > -char *acpi_ut_get_event_name(u32 event_id)
> > +const char *acpi_ut_get_event_name(u32 event_id)
> >  {
> > 
> >  	if (event_id > ACPI_EVENT_MAX) {
> >  		return ("InvalidEventID");
> >  	}
> > 
> > -	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
> > +	return (ACPI_CAST_PTR(const char, acpi_gbl_event_types[event_id]));
> >  }
> > 
> > 
> > /*************************************************************************
> > ******
> > @@ -220,17 +220,17 @@ static const char *acpi_gbl_ns_type_names[] = {
> >  	/* 30 */ "Invalid"
> >  };
> > 
> > -char *acpi_ut_get_type_name(acpi_object_type type)
> > +const char *acpi_ut_get_type_name(acpi_object_type type)
> >  {
> > 
> >  	if (type > ACPI_TYPE_INVALID) {
> > -		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
> > +		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
> >  	}
> > 
> > -	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
> > +	return (ACPI_CAST_PTR(const char, acpi_gbl_ns_type_names[type]));
> >  }
> > 
> > -char *acpi_ut_get_object_type_name(union acpi_operand_object *obj_desc)
> > +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> > +*obj_desc)
> >  {
> > 
> >  	if (!obj_desc) {
> > @@ -318,7 +318,7 @@ static const char *acpi_gbl_desc_type_names[] = {
> >  	/* 15 */ "Node"
> >  };
> > 
> > -char *acpi_ut_get_descriptor_name(void *object)
> > +const char *acpi_ut_get_descriptor_name(void *object)
> >  {
> > 
> >  	if (!object) {
> > @@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void *object)
> >  		return ("Not a Descriptor");
> >  	}
> > 
> > -	return (ACPI_CAST_PTR(char,
> > +	return (ACPI_CAST_PTR(const char,
> >  			      acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
> >  						       (object)]));
> > 
> > @@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union
> > acpi_operand_object *object)
> > 
> >  /* Names for internal mutex objects, used for debug output */
> > 
> > -static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > +static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> >  	"ACPI_MTX_Interpreter",
> >  	"ACPI_MTX_Namespace",
> >  	"ACPI_MTX_Tables",
> > @@ -411,7 +411,7 @@ static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> >  	"ACPI_MTX_CommandReady"
> >  };
> > 
> > -char *acpi_ut_get_mutex_name(u32 mutex_id)
> > +const char *acpi_ut_get_mutex_name(u32 mutex_id)
> >  {
> > 
> >  	if (mutex_id > ACPI_MAX_MUTEX) {
> > diff --git a/drivers/acpi/acpica/uthex.c b/drivers/acpi/acpica/uthex.c
> > index fda8b3d..9239711 100644
> > --- a/drivers/acpi/acpica/uthex.c
> > +++ b/drivers/acpi/acpica/uthex.c
> > @@ -48,7 +48,7 @@
> >  ACPI_MODULE_NAME("uthex")
> > 
> >  /* Hex to ASCII conversion table */
> > -static char acpi_gbl_hex_to_ascii[] = {
> > +static const char acpi_gbl_hex_to_ascii[] = {
> >  	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
> > 'D',
> >  	    'E', 'F'
> >  };
> > @@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
> >   *
> > 
> > **************************************************************************
> > ****/
> > 
> > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> >  {
> > 
> >  	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]); diff --
> > git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 17a6fa0..7c8a037
> > 100644
> > --- a/drivers/acpi/tables.c
> > +++ b/drivers/acpi/tables.c
> > @@ -35,8 +35,10 @@
> > 
> >  #define ACPI_MAX_TABLES		128
> > 
> > -static char *mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
> > -static char *mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level"
> > };
> > +static const char * const mps_inti_flags_polarity[] = {
> > +	"dfl", "high", "res", "low" };
> > +static const char * const mps_inti_flags_trigger[] = {
> > +	"dfl", "edge", "res", "level" };
> > 
> >  static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES] __initdata;
> > 
> > --
> > 2.4.9
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] acpi: set return value to const char for some functions
  2015-10-19 12:02   ` LABBE Corentin
@ 2015-10-19 17:04     ` Moore, Robert
  2015-10-19 17:15     ` Moore, Robert
  1 sibling, 0 replies; 12+ messages in thread
From: Moore, Robert @ 2015-10-19 17:04 UTC (permalink / raw)
  To: LABBE Corentin
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org

Actually, I thought I got most of the changes.

Again, ACPICA code is in a different format than the Linux version of the code, so your patch cannot be directly applied.

Bob


> -----Original Message-----
> From: LABBE Corentin [mailto:montjoie.mailing@gmail.com]
> Sent: Monday, October 19, 2015 5:03 AM
> To: Moore, Robert
> Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org; linux-
> acpi@vger.kernel.org; devel@acpica.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] acpi: set return value to const char for some
> functions
> 
> On Wed, Oct 14, 2015 at 08:53:19PM +0000, Moore, Robert wrote:
> > In ACPICA, we tend to be very careful concerning the "const" keyword in
> order to avoid a phenomenon known as "const pollution".
> >
> > That is not to say that we won't use const in some limited cases.
> >
> > Bob
> >
> 
> Hello
> 
> The problem is that all function in this patch return a pointer to string
> literal without giving that information.
> I am sad to see that you seems to agree with that but, for resuming your
> conversation with Joe Perches, you reject this patch just because you
> worry that perhaps in the future it will make you more works do deal with
> it.
> 
> Does you will accept the subset of this patch for adding const to "static
> char xxx[] = {}" ?
> 
> Regards
> 
> >
> > > -----Original Message-----
> > > From: LABBE Corentin [mailto:clabbe.montjoie@gmail.com]
> > > Sent: Wednesday, October 14, 2015 12:07 PM
> > > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org
> > > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > > kernel@vger.kernel.org; LABBE Corentin
> > > Subject: [PATCH] acpi: set return value to const char for some
> > > functions
> > >
> > > This patch set some array of const char as const.
> > > In the same time, some function return pointer to thoses array
> > > without properly giving the information that the data is const.
> > > This patch set the return type of thoses functions as const char *
> > >
> > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > ---
> > >  drivers/acpi/acpica/acutils.h  | 12 ++++++------
> > > drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
> > >  drivers/acpi/acpica/uthex.c    |  4 ++--
> > >  drivers/acpi/tables.c          |  6 ++++--
> > >  4 files changed, 24 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/acpi/acpica/acutils.h
> > > b/drivers/acpi/acpica/acutils.h index fb2aa50..e5d9d4f 100644
> > > --- a/drivers/acpi/acpica/acutils.h
> > > +++ b/drivers/acpi/acpica/acutils.h
> > > @@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32 mutex_id);
> > > const char *acpi_ut_get_notify_name(u32 notify_value,
> > > acpi_object_type type); #endif
> > >
> > > -char *acpi_ut_get_type_name(acpi_object_type type);
> > > +const char *acpi_ut_get_type_name(acpi_object_type type);
> > >
> > >  char *acpi_ut_get_node_name(void *object);
> > >
> > > -char *acpi_ut_get_descriptor_name(void *object);
> > > +const char *acpi_ut_get_descriptor_name(void *object);
> > >
> > >  const char *acpi_ut_get_reference_name(union acpi_operand_object
> > > *object);
> > >
> > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > *obj_desc);
> > > +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > +*obj_desc);
> > >
> > > -char *acpi_ut_get_region_name(u8 space_id);
> > > +const char *acpi_ut_get_region_name(u8 space_id);
> > >
> > > -char *acpi_ut_get_event_name(u32 event_id);
> > > +const char *acpi_ut_get_event_name(u32 event_id);
> > >
> > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > >
> > >  u8 acpi_ut_ascii_char_to_hex(int hex_char);
> > >
> > > diff --git a/drivers/acpi/acpica/utdecode.c
> > > b/drivers/acpi/acpica/utdecode.c index 988e23b..e08cdb1 100644
> > > --- a/drivers/acpi/acpica/utdecode.c
> > > +++ b/drivers/acpi/acpica/utdecode.c
> > > @@ -114,7 +114,7 @@ const char
> > > *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
> > >  	"PCC"			/* 0x0A */
> > >  };
> > >
> > > -char *acpi_ut_get_region_name(u8 space_id)
> > > +const char *acpi_ut_get_region_name(u8 space_id)
> > >  {
> > >
> > >  	if (space_id >= ACPI_USER_REGION_BEGIN) { @@ -127,7 +127,7 @@ char
> > > *acpi_ut_get_region_name(u8 space_id)
> > >  		return ("InvalidSpaceId");
> > >  	}
> > >
> > > -	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
> > > +	return (ACPI_CAST_PTR(const char,
> > > +acpi_gbl_region_types[space_id]));
> > >  }
> > >
> > >
> > > /*******************************************************************
> > > ******
> > > ******
> > > @@ -152,14 +152,14 @@ static const char
> > > *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
> > >  	"RealTimeClock",
> > >  };
> > >
> > > -char *acpi_ut_get_event_name(u32 event_id)
> > > +const char *acpi_ut_get_event_name(u32 event_id)
> > >  {
> > >
> > >  	if (event_id > ACPI_EVENT_MAX) {
> > >  		return ("InvalidEventID");
> > >  	}
> > >
> > > -	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
> > > +	return (ACPI_CAST_PTR(const char,
> > > +acpi_gbl_event_types[event_id]));
> > >  }
> > >
> > >
> > > /*******************************************************************
> > > ******
> > > ******
> > > @@ -220,17 +220,17 @@ static const char *acpi_gbl_ns_type_names[] = {
> > >  	/* 30 */ "Invalid"
> > >  };
> > >
> > > -char *acpi_ut_get_type_name(acpi_object_type type)
> > > +const char *acpi_ut_get_type_name(acpi_object_type type)
> > >  {
> > >
> > >  	if (type > ACPI_TYPE_INVALID) {
> > > -		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
> > > +		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
> > >  	}
> > >
> > > -	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
> > > +	return (ACPI_CAST_PTR(const char, acpi_gbl_ns_type_names[type]));
> > >  }
> > >
> > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > *obj_desc)
> > > +const char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > +*obj_desc)
> > >  {
> > >
> > >  	if (!obj_desc) {
> > > @@ -318,7 +318,7 @@ static const char *acpi_gbl_desc_type_names[] = {
> > >  	/* 15 */ "Node"
> > >  };
> > >
> > > -char *acpi_ut_get_descriptor_name(void *object)
> > > +const char *acpi_ut_get_descriptor_name(void *object)
> > >  {
> > >
> > >  	if (!object) {
> > > @@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void *object)
> > >  		return ("Not a Descriptor");
> > >  	}
> > >
> > > -	return (ACPI_CAST_PTR(char,
> > > +	return (ACPI_CAST_PTR(const char,
> > >  			      acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
> > >  						       (object)]));
> > >
> > > @@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union
> > > acpi_operand_object *object)
> > >
> > >  /* Names for internal mutex objects, used for debug output */
> > >
> > > -static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > > +static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > >  	"ACPI_MTX_Interpreter",
> > >  	"ACPI_MTX_Namespace",
> > >  	"ACPI_MTX_Tables",
> > > @@ -411,7 +411,7 @@ static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX]
> = {
> > >  	"ACPI_MTX_CommandReady"
> > >  };
> > >
> > > -char *acpi_ut_get_mutex_name(u32 mutex_id)
> > > +const char *acpi_ut_get_mutex_name(u32 mutex_id)
> > >  {
> > >
> > >  	if (mutex_id > ACPI_MAX_MUTEX) {
> > > diff --git a/drivers/acpi/acpica/uthex.c
> > > b/drivers/acpi/acpica/uthex.c index fda8b3d..9239711 100644
> > > --- a/drivers/acpi/acpica/uthex.c
> > > +++ b/drivers/acpi/acpica/uthex.c
> > > @@ -48,7 +48,7 @@
> > >  ACPI_MODULE_NAME("uthex")
> > >
> > >  /* Hex to ASCII conversion table */ -static char
> > > acpi_gbl_hex_to_ascii[] = {
> > > +static const char acpi_gbl_hex_to_ascii[] = {
> > >  	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
> > > 'D',
> > >  	    'E', 'F'
> > >  };
> > > @@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
> > >   *
> > >
> > > ********************************************************************
> > > ******
> > > ****/
> > >
> > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > >  {
> > >
> > >  	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]); diff
> > > -- git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index
> > > 17a6fa0..7c8a037
> > > 100644
> > > --- a/drivers/acpi/tables.c
> > > +++ b/drivers/acpi/tables.c
> > > @@ -35,8 +35,10 @@
> > >
> > >  #define ACPI_MAX_TABLES		128
> > >
> > > -static char *mps_inti_flags_polarity[] = { "dfl", "high", "res",
> > > "low" }; -static char *mps_inti_flags_trigger[] = { "dfl", "edge",
> "res", "level"
> > > };
> > > +static const char * const mps_inti_flags_polarity[] = {
> > > +	"dfl", "high", "res", "low" };
> > > +static const char * const mps_inti_flags_trigger[] = {
> > > +	"dfl", "edge", "res", "level" };
> > >
> > >  static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES]
> > > __initdata;
> > >
> > > --
> > > 2.4.9
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-kernel" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] acpi: set return value to const char for some functions
  2015-10-19 12:02   ` LABBE Corentin
  2015-10-19 17:04     ` Moore, Robert
@ 2015-10-19 17:15     ` Moore, Robert
  2015-10-21 17:44       ` [Devel] " Moore, Robert
  1 sibling, 1 reply; 12+ messages in thread
From: Moore, Robert @ 2015-10-19 17:15 UTC (permalink / raw)
  To: LABBE Corentin
  Cc: LABBE Corentin, Zheng, Lv, Wysocki, Rafael J, lenb@kernel.org,
	linux-acpi@vger.kernel.org, devel@acpica.org,
	linux-kernel@vger.kernel.org

You may have missed this, from last week:


> All that being said, I went ahead and made the actual ACPICA changes
> corresponding to your patches. There were no pollution issues, and it
> actually simplified the code by eliminating the need for some uses of
> ACPI_CAST_PTR.
> 
> The only issue I found was here, so we won't do this one:
> 
> const char
> AcpiUtHexToAsciiChar
> 
> \acpica\source\include\acutils.h(322) :
>     warning C4180: qualifier applied to function type has no meaning;
> ignored
>







> -----Original Message-----
> From: Moore, Robert
> Sent: Monday, October 19, 2015 10:05 AM
> To: 'LABBE Corentin'
> Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org; linux-
> acpi@vger.kernel.org; devel@acpica.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] acpi: set return value to const char for some
> functions
> 
> Actually, I thought I got most of the changes.
> 
> Again, ACPICA code is in a different format than the Linux version of the
> code, so your patch cannot be directly applied.
> 
> Bob
> 
> 
> > -----Original Message-----
> > From: LABBE Corentin [mailto:montjoie.mailing@gmail.com]
> > Sent: Monday, October 19, 2015 5:03 AM
> > To: Moore, Robert
> > Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org;
> > linux- acpi@vger.kernel.org; devel@acpica.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] acpi: set return value to const char for some
> > functions
> >
> > On Wed, Oct 14, 2015 at 08:53:19PM +0000, Moore, Robert wrote:
> > > In ACPICA, we tend to be very careful concerning the "const" keyword
> > > in
> > order to avoid a phenomenon known as "const pollution".
> > >
> > > That is not to say that we won't use const in some limited cases.
> > >
> > > Bob
> > >
> >
> > Hello
> >
> > The problem is that all function in this patch return a pointer to
> > string literal without giving that information.
> > I am sad to see that you seems to agree with that but, for resuming
> > your conversation with Joe Perches, you reject this patch just because
> > you worry that perhaps in the future it will make you more works do
> > deal with it.
> >
> > Does you will accept the subset of this patch for adding const to
> > "static char xxx[] = {}" ?
> >
> > Regards
> >
> > >
> > > > -----Original Message-----
> > > > From: LABBE Corentin [mailto:clabbe.montjoie@gmail.com]
> > > > Sent: Wednesday, October 14, 2015 12:07 PM
> > > > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org
> > > > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > > > kernel@vger.kernel.org; LABBE Corentin
> > > > Subject: [PATCH] acpi: set return value to const char for some
> > > > functions
> > > >
> > > > This patch set some array of const char as const.
> > > > In the same time, some function return pointer to thoses array
> > > > without properly giving the information that the data is const.
> > > > This patch set the return type of thoses functions as const char *
> > > >
> > > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > > ---
> > > >  drivers/acpi/acpica/acutils.h  | 12 ++++++------
> > > > drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
> > > >  drivers/acpi/acpica/uthex.c    |  4 ++--
> > > >  drivers/acpi/tables.c          |  6 ++++--
> > > >  4 files changed, 24 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/acpica/acutils.h
> > > > b/drivers/acpi/acpica/acutils.h index fb2aa50..e5d9d4f 100644
> > > > --- a/drivers/acpi/acpica/acutils.h
> > > > +++ b/drivers/acpi/acpica/acutils.h
> > > > @@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32 mutex_id);
> > > > const char *acpi_ut_get_notify_name(u32 notify_value,
> > > > acpi_object_type type); #endif
> > > >
> > > > -char *acpi_ut_get_type_name(acpi_object_type type);
> > > > +const char *acpi_ut_get_type_name(acpi_object_type type);
> > > >
> > > >  char *acpi_ut_get_node_name(void *object);
> > > >
> > > > -char *acpi_ut_get_descriptor_name(void *object);
> > > > +const char *acpi_ut_get_descriptor_name(void *object);
> > > >
> > > >  const char *acpi_ut_get_reference_name(union acpi_operand_object
> > > > *object);
> > > >
> > > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > > *obj_desc);
> > > > +const char *acpi_ut_get_object_type_name(union
> > > > +acpi_operand_object *obj_desc);
> > > >
> > > > -char *acpi_ut_get_region_name(u8 space_id);
> > > > +const char *acpi_ut_get_region_name(u8 space_id);
> > > >
> > > > -char *acpi_ut_get_event_name(u32 event_id);
> > > > +const char *acpi_ut_get_event_name(u32 event_id);
> > > >
> > > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > > >
> > > >  u8 acpi_ut_ascii_char_to_hex(int hex_char);
> > > >
> > > > diff --git a/drivers/acpi/acpica/utdecode.c
> > > > b/drivers/acpi/acpica/utdecode.c index 988e23b..e08cdb1 100644
> > > > --- a/drivers/acpi/acpica/utdecode.c
> > > > +++ b/drivers/acpi/acpica/utdecode.c
> > > > @@ -114,7 +114,7 @@ const char
> > > > *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
> > > >  	"PCC"			/* 0x0A */
> > > >  };
> > > >
> > > > -char *acpi_ut_get_region_name(u8 space_id)
> > > > +const char *acpi_ut_get_region_name(u8 space_id)
> > > >  {
> > > >
> > > >  	if (space_id >= ACPI_USER_REGION_BEGIN) { @@ -127,7 +127,7 @@
> > > > char
> > > > *acpi_ut_get_region_name(u8 space_id)
> > > >  		return ("InvalidSpaceId");
> > > >  	}
> > > >
> > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
> > > > +	return (ACPI_CAST_PTR(const char,
> > > > +acpi_gbl_region_types[space_id]));
> > > >  }
> > > >
> > > >
> > > > /*****************************************************************
> > > > **
> > > > ******
> > > > ******
> > > > @@ -152,14 +152,14 @@ static const char
> > > > *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
> > > >  	"RealTimeClock",
> > > >  };
> > > >
> > > > -char *acpi_ut_get_event_name(u32 event_id)
> > > > +const char *acpi_ut_get_event_name(u32 event_id)
> > > >  {
> > > >
> > > >  	if (event_id > ACPI_EVENT_MAX) {
> > > >  		return ("InvalidEventID");
> > > >  	}
> > > >
> > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
> > > > +	return (ACPI_CAST_PTR(const char,
> > > > +acpi_gbl_event_types[event_id]));
> > > >  }
> > > >
> > > >
> > > > /*****************************************************************
> > > > **
> > > > ******
> > > > ******
> > > > @@ -220,17 +220,17 @@ static const char *acpi_gbl_ns_type_names[] =
> {
> > > >  	/* 30 */ "Invalid"
> > > >  };
> > > >
> > > > -char *acpi_ut_get_type_name(acpi_object_type type)
> > > > +const char *acpi_ut_get_type_name(acpi_object_type type)
> > > >  {
> > > >
> > > >  	if (type > ACPI_TYPE_INVALID) {
> > > > -		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
> > > > +		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
> > > >  	}
> > > >
> > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
> > > > +	return (ACPI_CAST_PTR(const char,
> > > > +acpi_gbl_ns_type_names[type]));
> > > >  }
> > > >
> > > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > > *obj_desc)
> > > > +const char *acpi_ut_get_object_type_name(union
> > > > +acpi_operand_object
> > > > +*obj_desc)
> > > >  {
> > > >
> > > >  	if (!obj_desc) {
> > > > @@ -318,7 +318,7 @@ static const char *acpi_gbl_desc_type_names[] =
> {
> > > >  	/* 15 */ "Node"
> > > >  };
> > > >
> > > > -char *acpi_ut_get_descriptor_name(void *object)
> > > > +const char *acpi_ut_get_descriptor_name(void *object)
> > > >  {
> > > >
> > > >  	if (!object) {
> > > > @@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void *object)
> > > >  		return ("Not a Descriptor");
> > > >  	}
> > > >
> > > > -	return (ACPI_CAST_PTR(char,
> > > > +	return (ACPI_CAST_PTR(const char,
> > > >
> acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
> > > >  						       (object)]));
> > > >
> > > > @@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union
> > > > acpi_operand_object *object)
> > > >
> > > >  /* Names for internal mutex objects, used for debug output */
> > > >
> > > > -static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > > > +static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > > >  	"ACPI_MTX_Interpreter",
> > > >  	"ACPI_MTX_Namespace",
> > > >  	"ACPI_MTX_Tables",
> > > > @@ -411,7 +411,7 @@ static char
> > > > *acpi_gbl_mutex_names[ACPI_NUM_MUTEX]
> > = {
> > > >  	"ACPI_MTX_CommandReady"
> > > >  };
> > > >
> > > > -char *acpi_ut_get_mutex_name(u32 mutex_id)
> > > > +const char *acpi_ut_get_mutex_name(u32 mutex_id)
> > > >  {
> > > >
> > > >  	if (mutex_id > ACPI_MAX_MUTEX) { diff --git
> > > > a/drivers/acpi/acpica/uthex.c b/drivers/acpi/acpica/uthex.c index
> > > > fda8b3d..9239711 100644
> > > > --- a/drivers/acpi/acpica/uthex.c
> > > > +++ b/drivers/acpi/acpica/uthex.c
> > > > @@ -48,7 +48,7 @@
> > > >  ACPI_MODULE_NAME("uthex")
> > > >
> > > >  /* Hex to ASCII conversion table */ -static char
> > > > acpi_gbl_hex_to_ascii[] = {
> > > > +static const char acpi_gbl_hex_to_ascii[] = {
> > > >  	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B',
> 'C',
> > > > 'D',
> > > >  	    'E', 'F'
> > > >  };
> > > > @@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
> > > >   *
> > > >
> > > > ******************************************************************
> > > > **
> > > > ******
> > > > ****/
> > > >
> > > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > > >  {
> > > >
> > > >  	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]);
> > > > diff
> > > > -- git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index
> > > > 17a6fa0..7c8a037
> > > > 100644
> > > > --- a/drivers/acpi/tables.c
> > > > +++ b/drivers/acpi/tables.c
> > > > @@ -35,8 +35,10 @@
> > > >
> > > >  #define ACPI_MAX_TABLES		128
> > > >
> > > > -static char *mps_inti_flags_polarity[] = { "dfl", "high", "res",
> > > > "low" }; -static char *mps_inti_flags_trigger[] = { "dfl", "edge",
> > "res", "level"
> > > > };
> > > > +static const char * const mps_inti_flags_polarity[] = {
> > > > +	"dfl", "high", "res", "low" };
> > > > +static const char * const mps_inti_flags_trigger[] = {
> > > > +	"dfl", "edge", "res", "level" };
> > > >
> > > >  static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES]
> > > > __initdata;
> > > >
> > > > --
> > > > 2.4.9
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > > linux-kernel" in the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [Devel] [PATCH] acpi: set return value to const char for some functions
  2015-10-19 17:15     ` Moore, Robert
@ 2015-10-21 17:44       ` Moore, Robert
  0 siblings, 0 replies; 12+ messages in thread
From: Moore, Robert @ 2015-10-21 17:44 UTC (permalink / raw)
  To: Moore, Robert, LABBE Corentin
  Cc: linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
	LABBE Corentin, devel@acpica.org

I think we have everything applied. As I said, we try to be careful about const, but that doesn't mean we are against it.

So, thanks for your help.
Bob


> -----Original Message-----
> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Moore, Robert
> Sent: Monday, October 19, 2015 10:16 AM
> To: LABBE Corentin
> Cc: Wysocki, Rafael J; linux-kernel@vger.kernel.org; linux-
> acpi@vger.kernel.org; LABBE Corentin; devel@acpica.org
> Subject: Re: [Devel] [PATCH] acpi: set return value to const char for some
> functions
> 
> You may have missed this, from last week:
> 
> 
> > All that being said, I went ahead and made the actual ACPICA changes
> > corresponding to your patches. There were no pollution issues, and it
> > actually simplified the code by eliminating the need for some uses of
> > ACPI_CAST_PTR.
> >
> > The only issue I found was here, so we won't do this one:
> >
> > const char
> > AcpiUtHexToAsciiChar
> >
> > \acpica\source\include\acutils.h(322) :
> >     warning C4180: qualifier applied to function type has no meaning;
> > ignored
> >
> 
> 
> 
> 
> 
> 
> 
> > -----Original Message-----
> > From: Moore, Robert
> > Sent: Monday, October 19, 2015 10:05 AM
> > To: 'LABBE Corentin'
> > Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org;
> > linux- acpi@vger.kernel.org; devel@acpica.org;
> > linux-kernel@vger.kernel.org
> > Subject: RE: [PATCH] acpi: set return value to const char for some
> > functions
> >
> > Actually, I thought I got most of the changes.
> >
> > Again, ACPICA code is in a different format than the Linux version of
> > the code, so your patch cannot be directly applied.
> >
> > Bob
> >
> >
> > > -----Original Message-----
> > > From: LABBE Corentin [mailto:montjoie.mailing@gmail.com]
> > > Sent: Monday, October 19, 2015 5:03 AM
> > > To: Moore, Robert
> > > Cc: LABBE Corentin; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org;
> > > linux- acpi@vger.kernel.org; devel@acpica.org;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] acpi: set return value to const char for some
> > > functions
> > >
> > > On Wed, Oct 14, 2015 at 08:53:19PM +0000, Moore, Robert wrote:
> > > > In ACPICA, we tend to be very careful concerning the "const"
> > > > keyword in
> > > order to avoid a phenomenon known as "const pollution".
> > > >
> > > > That is not to say that we won't use const in some limited cases.
> > > >
> > > > Bob
> > > >
> > >
> > > Hello
> > >
> > > The problem is that all function in this patch return a pointer to
> > > string literal without giving that information.
> > > I am sad to see that you seems to agree with that but, for resuming
> > > your conversation with Joe Perches, you reject this patch just
> > > because you worry that perhaps in the future it will make you more
> > > works do deal with it.
> > >
> > > Does you will accept the subset of this patch for adding const to
> > > "static char xxx[] = {}" ?
> > >
> > > Regards
> > >
> > > >
> > > > > -----Original Message-----
> > > > > From: LABBE Corentin [mailto:clabbe.montjoie@gmail.com]
> > > > > Sent: Wednesday, October 14, 2015 12:07 PM
> > > > > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; lenb@kernel.org
> > > > > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > > > > kernel@vger.kernel.org; LABBE Corentin
> > > > > Subject: [PATCH] acpi: set return value to const char for some
> > > > > functions
> > > > >
> > > > > This patch set some array of const char as const.
> > > > > In the same time, some function return pointer to thoses array
> > > > > without properly giving the information that the data is const.
> > > > > This patch set the return type of thoses functions as const char
> > > > > *
> > > > >
> > > > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > > > ---
> > > > >  drivers/acpi/acpica/acutils.h  | 12 ++++++------
> > > > > drivers/acpi/acpica/utdecode.c | 24 ++++++++++++------------
> > > > >  drivers/acpi/acpica/uthex.c    |  4 ++--
> > > > >  drivers/acpi/tables.c          |  6 ++++--
> > > > >  4 files changed, 24 insertions(+), 22 deletions(-)
> > > > >
> > > > > diff --git a/drivers/acpi/acpica/acutils.h
> > > > > b/drivers/acpi/acpica/acutils.h index fb2aa50..e5d9d4f 100644
> > > > > --- a/drivers/acpi/acpica/acutils.h
> > > > > +++ b/drivers/acpi/acpica/acutils.h
> > > > > @@ -189,21 +189,21 @@ char *acpi_ut_get_mutex_name(u32
> > > > > mutex_id); const char *acpi_ut_get_notify_name(u32 notify_value,
> > > > > acpi_object_type type); #endif
> > > > >
> > > > > -char *acpi_ut_get_type_name(acpi_object_type type);
> > > > > +const char *acpi_ut_get_type_name(acpi_object_type type);
> > > > >
> > > > >  char *acpi_ut_get_node_name(void *object);
> > > > >
> > > > > -char *acpi_ut_get_descriptor_name(void *object);
> > > > > +const char *acpi_ut_get_descriptor_name(void *object);
> > > > >
> > > > >  const char *acpi_ut_get_reference_name(union
> > > > > acpi_operand_object *object);
> > > > >
> > > > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > > > *obj_desc);
> > > > > +const char *acpi_ut_get_object_type_name(union
> > > > > +acpi_operand_object *obj_desc);
> > > > >
> > > > > -char *acpi_ut_get_region_name(u8 space_id);
> > > > > +const char *acpi_ut_get_region_name(u8 space_id);
> > > > >
> > > > > -char *acpi_ut_get_event_name(u32 event_id);
> > > > > +const char *acpi_ut_get_event_name(u32 event_id);
> > > > >
> > > > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
> > > > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32
> > > > > +position);
> > > > >
> > > > >  u8 acpi_ut_ascii_char_to_hex(int hex_char);
> > > > >
> > > > > diff --git a/drivers/acpi/acpica/utdecode.c
> > > > > b/drivers/acpi/acpica/utdecode.c index 988e23b..e08cdb1 100644
> > > > > --- a/drivers/acpi/acpica/utdecode.c
> > > > > +++ b/drivers/acpi/acpica/utdecode.c
> > > > > @@ -114,7 +114,7 @@ const char
> > > > > *acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] = {
> > > > >  	"PCC"			/* 0x0A */
> > > > >  };
> > > > >
> > > > > -char *acpi_ut_get_region_name(u8 space_id)
> > > > > +const char *acpi_ut_get_region_name(u8 space_id)
> > > > >  {
> > > > >
> > > > >  	if (space_id >= ACPI_USER_REGION_BEGIN) { @@ -127,7 +127,7 @@
> > > > > char
> > > > > *acpi_ut_get_region_name(u8 space_id)
> > > > >  		return ("InvalidSpaceId");
> > > > >  	}
> > > > >
> > > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_region_types[space_id]));
> > > > > +	return (ACPI_CAST_PTR(const char,
> > > > > +acpi_gbl_region_types[space_id]));
> > > > >  }
> > > > >
> > > > >
> > > > > /***************************************************************
> > > > > **
> > > > > **
> > > > > ******
> > > > > ******
> > > > > @@ -152,14 +152,14 @@ static const char
> > > > > *acpi_gbl_event_types[ACPI_NUM_FIXED_EVENTS] = {
> > > > >  	"RealTimeClock",
> > > > >  };
> > > > >
> > > > > -char *acpi_ut_get_event_name(u32 event_id)
> > > > > +const char *acpi_ut_get_event_name(u32 event_id)
> > > > >  {
> > > > >
> > > > >  	if (event_id > ACPI_EVENT_MAX) {
> > > > >  		return ("InvalidEventID");
> > > > >  	}
> > > > >
> > > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_event_types[event_id]));
> > > > > +	return (ACPI_CAST_PTR(const char,
> > > > > +acpi_gbl_event_types[event_id]));
> > > > >  }
> > > > >
> > > > >
> > > > > /***************************************************************
> > > > > **
> > > > > **
> > > > > ******
> > > > > ******
> > > > > @@ -220,17 +220,17 @@ static const char
> > > > > *acpi_gbl_ns_type_names[] =
> > {
> > > > >  	/* 30 */ "Invalid"
> > > > >  };
> > > > >
> > > > > -char *acpi_ut_get_type_name(acpi_object_type type)
> > > > > +const char *acpi_ut_get_type_name(acpi_object_type type)
> > > > >  {
> > > > >
> > > > >  	if (type > ACPI_TYPE_INVALID) {
> > > > > -		return (ACPI_CAST_PTR(char, acpi_gbl_bad_type));
> > > > > +		return (ACPI_CAST_PTR(const char, acpi_gbl_bad_type));
> > > > >  	}
> > > > >
> > > > > -	return (ACPI_CAST_PTR(char, acpi_gbl_ns_type_names[type]));
> > > > > +	return (ACPI_CAST_PTR(const char,
> > > > > +acpi_gbl_ns_type_names[type]));
> > > > >  }
> > > > >
> > > > > -char *acpi_ut_get_object_type_name(union acpi_operand_object
> > > > > *obj_desc)
> > > > > +const char *acpi_ut_get_object_type_name(union
> > > > > +acpi_operand_object
> > > > > +*obj_desc)
> > > > >  {
> > > > >
> > > > >  	if (!obj_desc) {
> > > > > @@ -318,7 +318,7 @@ static const char
> > > > > *acpi_gbl_desc_type_names[] =
> > {
> > > > >  	/* 15 */ "Node"
> > > > >  };
> > > > >
> > > > > -char *acpi_ut_get_descriptor_name(void *object)
> > > > > +const char *acpi_ut_get_descriptor_name(void *object)
> > > > >  {
> > > > >
> > > > >  	if (!object) {
> > > > > @@ -329,7 +329,7 @@ char *acpi_ut_get_descriptor_name(void
> *object)
> > > > >  		return ("Not a Descriptor");
> > > > >  	}
> > > > >
> > > > > -	return (ACPI_CAST_PTR(char,
> > > > > +	return (ACPI_CAST_PTR(const char,
> > > > >
> > acpi_gbl_desc_type_names[ACPI_GET_DESCRIPTOR_TYPE
> > > > >  						       (object)]));
> > > > >
> > > > > @@ -400,7 +400,7 @@ const char *acpi_ut_get_reference_name(union
> > > > > acpi_operand_object *object)
> > > > >
> > > > >  /* Names for internal mutex objects, used for debug output */
> > > > >
> > > > > -static char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > > > > +static const char *acpi_gbl_mutex_names[ACPI_NUM_MUTEX] = {
> > > > >  	"ACPI_MTX_Interpreter",
> > > > >  	"ACPI_MTX_Namespace",
> > > > >  	"ACPI_MTX_Tables",
> > > > > @@ -411,7 +411,7 @@ static char
> > > > > *acpi_gbl_mutex_names[ACPI_NUM_MUTEX]
> > > = {
> > > > >  	"ACPI_MTX_CommandReady"
> > > > >  };
> > > > >
> > > > > -char *acpi_ut_get_mutex_name(u32 mutex_id)
> > > > > +const char *acpi_ut_get_mutex_name(u32 mutex_id)
> > > > >  {
> > > > >
> > > > >  	if (mutex_id > ACPI_MAX_MUTEX) { diff --git
> > > > > a/drivers/acpi/acpica/uthex.c b/drivers/acpi/acpica/uthex.c
> > > > > index
> > > > > fda8b3d..9239711 100644
> > > > > --- a/drivers/acpi/acpica/uthex.c
> > > > > +++ b/drivers/acpi/acpica/uthex.c
> > > > > @@ -48,7 +48,7 @@
> > > > >  ACPI_MODULE_NAME("uthex")
> > > > >
> > > > >  /* Hex to ASCII conversion table */ -static char
> > > > > acpi_gbl_hex_to_ascii[] = {
> > > > > +static const char acpi_gbl_hex_to_ascii[] = {
> > > > >  	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B',
> > 'C',
> > > > > 'D',
> > > > >  	    'E', 'F'
> > > > >  };
> > > > > @@ -67,7 +67,7 @@ static char acpi_gbl_hex_to_ascii[] = {
> > > > >   *
> > > > >
> > > > > ****************************************************************
> > > > > **
> > > > > **
> > > > > ******
> > > > > ****/
> > > > >
> > > > > -char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > > > > +const char acpi_ut_hex_to_ascii_char(u64 integer, u32 position)
> > > > >  {
> > > > >
> > > > >  	return (acpi_gbl_hex_to_ascii[(integer >> position) & 0xF]);
> > > > > diff
> > > > > -- git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index
> > > > > 17a6fa0..7c8a037
> > > > > 100644
> > > > > --- a/drivers/acpi/tables.c
> > > > > +++ b/drivers/acpi/tables.c
> > > > > @@ -35,8 +35,10 @@
> > > > >
> > > > >  #define ACPI_MAX_TABLES		128
> > > > >
> > > > > -static char *mps_inti_flags_polarity[] = { "dfl", "high",
> > > > > "res", "low" }; -static char *mps_inti_flags_trigger[] = {
> > > > > "dfl", "edge",
> > > "res", "level"
> > > > > };
> > > > > +static const char * const mps_inti_flags_polarity[] = {
> > > > > +	"dfl", "high", "res", "low" }; static const char * const
> > > > > +mps_inti_flags_trigger[] = {
> > > > > +	"dfl", "edge", "res", "level" };
> > > > >
> > > > >  static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES]
> > > > > __initdata;
> > > > >
> > > > > --
> > > > > 2.4.9
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-kernel" in the body of a message to
> > > > majordomo@vger.kernel.org More majordomo info at
> > > > http://vger.kernel.org/majordomo-info.html
> > > > Please read the FAQ at  http://www.tux.org/lkml/
> _______________________________________________
> Devel mailing list
> Devel@acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-10-21 17:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-14 19:07 [PATCH] acpi: set return value to const char for some functions LABBE Corentin
2015-10-14 19:48 ` kbuild test robot
2015-10-14 20:53 ` Moore, Robert
2015-10-15  2:14   ` Joe Perches
2015-10-15 19:32     ` Moore, Robert
2015-10-15 23:59       ` Joe Perches
2015-10-16  3:37         ` Moore, Robert
2015-10-16  3:47           ` Joe Perches
2015-10-19 12:02   ` LABBE Corentin
2015-10-19 17:04     ` Moore, Robert
2015-10-19 17:15     ` Moore, Robert
2015-10-21 17:44       ` [Devel] " Moore, Robert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).