linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
@ 2012-12-07 22:42 Cong Ding
  2012-12-07 22:45 ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Cong Ding @ 2012-12-07 22:42 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	Masami Hiramatsu, linux-kernel
  Cc: Cong Ding


Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..d1d9cfa 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
-- 
1.7.4.5


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

* Re: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 22:42 [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Cong Ding
@ 2012-12-07 22:45 ` H. Peter Anvin
  2012-12-07 22:49   ` Cong Ding
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-07 22:45 UTC (permalink / raw)
  To: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu,
	linux-kernel

Patch description please?

Cong Ding <dinggnu@gmail.com> wrote:

>
>Signed-off-by: Cong Ding <dinggnu@gmail.com>
>---
> arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>b/arch/x86/tools/gen-insn-attr-x86.awk
>index ddcf39b..d1d9cfa 100644
>--- a/arch/x86/tools/gen-insn-attr-x86.awk
>+++ b/arch/x86/tools/gen-insn-attr-x86.awk
>@@ -356,7 +356,7 @@ END {
> 		exit 1
> 	# print escape opcode map's array
> 	print "/* Escape opcode map array */"
>-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
>\
>+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
> 	      "[INAT_LSTPFX_MAX + 1] = {"
> 	for (i = 0; i < geid; i++)
> 		for (j = 0; j < max_lprefix; j++)
>@@ -365,7 +365,7 @@ END {
> 	print "};\n"
> 	# print group opcode map's array
> 	print "/* Group opcode map array */"
>-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
>+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
> 	      "[INAT_LSTPFX_MAX + 1] = {"
> 	for (i = 0; i < ggid; i++)
> 		for (j = 0; j < max_lprefix; j++)

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* Re: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 22:45 ` H. Peter Anvin
@ 2012-12-07 22:49   ` Cong Ding
  2012-12-07 22:56     ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Cong Ding @ 2012-12-07 22:49 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
> Patch description please?
there are 2 consts in the definition of one variable

- cong
> 
> Cong Ding <dinggnu@gmail.com> wrote:
> 
> >
> >Signed-off-by: Cong Ding <dinggnu@gmail.com>
> >---
> > arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
> >b/arch/x86/tools/gen-insn-attr-x86.awk
> >index ddcf39b..d1d9cfa 100644
> >--- a/arch/x86/tools/gen-insn-attr-x86.awk
> >+++ b/arch/x86/tools/gen-insn-attr-x86.awk
> >@@ -356,7 +356,7 @@ END {
> > 		exit 1
> > 	# print escape opcode map's array
> > 	print "/* Escape opcode map array */"
> >-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
> >\
> >+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
> > 	      "[INAT_LSTPFX_MAX + 1] = {"
> > 	for (i = 0; i < geid; i++)
> > 		for (j = 0; j < max_lprefix; j++)
> >@@ -365,7 +365,7 @@ END {
> > 	print "};\n"
> > 	# print group opcode map's array
> > 	print "/* Group opcode map array */"
> >-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
> >+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
> > 	      "[INAT_LSTPFX_MAX + 1] = {"
> > 	for (i = 0; i < ggid; i++)
> > 		for (j = 0; j < max_lprefix; j++)
> 
> -- 
> Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* Re: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 22:49   ` Cong Ding
@ 2012-12-07 22:56     ` H. Peter Anvin
  2012-12-07 23:03       ` Cong Ding
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-07 22:56 UTC (permalink / raw)
  To: Cong Ding
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On 12/07/2012 02:49 PM, Cong Ding wrote:
> On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
>> Patch description please?
> there are 2 consts in the definition of one variable
>

Please put in an actual patch description.  The first line (subject 
line) is a title; the patch should make sense without it.

	-hpa



-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


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

* Re: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 22:56     ` H. Peter Anvin
@ 2012-12-07 23:03       ` Cong Ding
  2012-12-07 23:06         ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Cong Ding @ 2012-12-07 23:03 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On Fri, Dec 07, 2012 at 02:56:16PM -0800, H. Peter Anvin wrote:
> On 12/07/2012 02:49 PM, Cong Ding wrote:
> >On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
> >>Patch description please?
> >there are 2 consts in the definition of one variable
> >
> 
> Please put in an actual patch description.  The first line (subject
> line) is a title; the patch should make sense without it.
sorry for that. so like this is fine?

-cong

---
>From 1abfab824ed2dc0af6e283ea0b7a6c45541d4fd1 Mon Sep 17 00:00:00 2001
From: Cong Ding <dinggnu@gmail.com>
Date: Fri, 7 Dec 2012 22:41:09 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const

there are two const in the definition of one variable, we should delete one.

Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..d1d9cfa 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
-- 
1.7.4.5


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

* Re: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 23:03       ` Cong Ding
@ 2012-12-07 23:06         ` H. Peter Anvin
  2012-12-07 23:17           ` [PATCH v2] " Cong Ding
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-07 23:06 UTC (permalink / raw)
  To: Cong Ding
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On 12/07/2012 03:03 PM, Cong Ding wrote:
> On Fri, Dec 07, 2012 at 02:56:16PM -0800, H. Peter Anvin wrote:
>> On 12/07/2012 02:49 PM, Cong Ding wrote:
>>> On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
>>>> Patch description please?
>>> there are 2 consts in the definition of one variable
>>>
>>
>> Please put in an actual patch description.  The first line (subject
>> line) is a title; the patch should make sense without it.
> sorry for that. so like this is fine?
>

Well, except that typically you should explain which variable it is. 
Yes, it is obvious if you look at the patch, but you're making the 
reader spend a few more moments than necessary.

Also, you should explain what the harm is -- if it breaks anything or is 
just a cosmetic issue.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


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

* [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 23:06         ` H. Peter Anvin
@ 2012-12-07 23:17           ` Cong Ding
  2012-12-07 23:28             ` H. Peter Anvin
                               ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Cong Ding @ 2012-12-07 23:17 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On Fri, Dec 07, 2012 at 03:06:13PM -0800, H. Peter Anvin wrote:
> On 12/07/2012 03:03 PM, Cong Ding wrote:
> >On Fri, Dec 07, 2012 at 02:56:16PM -0800, H. Peter Anvin wrote:
> >>On 12/07/2012 02:49 PM, Cong Ding wrote:
> >>>On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
> >>>>Patch description please?
> >>>there are 2 consts in the definition of one variable
> >>>
> >>
> >>Please put in an actual patch description.  The first line (subject
> >>line) is a title; the patch should make sense without it.
> >sorry for that. so like this is fine?
> >
> 
> Well, except that typically you should explain which variable it is.
> Yes, it is obvious if you look at the patch, but you're making the
> reader spend a few more moments than necessary.
> 
> Also, you should explain what the harm is -- if it breaks anything
> or is just a cosmetic issue.
sorry again for lacking of experience...
and I missed another same error, so send version 2.

- cong
---
>From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00 2001
From: Cong Ding <dinggnu@gmail.com>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const

fix the following sparse warning:
arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const

for variable inat_escape_tables, inat_group_tables, and inat_avx_tables

Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..987c7b2 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -374,7 +374,7 @@ END {
 	print "};\n"
 	# print AVX opcode map's array
 	print "/* AVX opcode map array */"
-	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < gaid; i++)
 		for (j = 0; j < max_lprefix; j++)
-- 
1.7.4.5


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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 23:17           ` [PATCH v2] " Cong Ding
@ 2012-12-07 23:28             ` H. Peter Anvin
  2012-12-08  0:04             ` [tip:x86/cleanups] x86: Remove duplicate "const" in gen-insn-attr-x86.awk tip-bot for Cong Ding
  2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
  2 siblings, 0 replies; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-07 23:28 UTC (permalink / raw)
  To: Cong Ding
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masami Hiramatsu, linux-kernel

On 12/07/2012 03:17 PM, Cong Ding wrote:
> On Fri, Dec 07, 2012 at 03:06:13PM -0800, H. Peter Anvin wrote:
>> On 12/07/2012 03:03 PM, Cong Ding wrote:
>>> On Fri, Dec 07, 2012 at 02:56:16PM -0800, H. Peter Anvin wrote:
>>>> On 12/07/2012 02:49 PM, Cong Ding wrote:
>>>>> On Fri, Dec 07, 2012 at 02:45:43PM -0800, H. Peter Anvin wrote:
>>>>>> Patch description please?
>>>>> there are 2 consts in the definition of one variable
>>>>>
>>>>
>>>> Please put in an actual patch description.  The first line (subject
>>>> line) is a title; the patch should make sense without it.
>>> sorry for that. so like this is fine?
>>>
>>
>> Well, except that typically you should explain which variable it is.
>> Yes, it is obvious if you look at the patch, but you're making the
>> reader spend a few more moments than necessary.
>>
>> Also, you should explain what the harm is -- if it breaks anything
>> or is just a cosmetic issue.
> sorry again for lacking of experience...
> and I missed another same error, so send version 2.
>

And one final complaint (I'll fix this one, but for the future):

git automation wants you to put commentary *after* the patch (after the 
line with three dashes) rather than before.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


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

* [tip:x86/cleanups] x86: Remove duplicate "const" in gen-insn-attr-x86.awk
  2012-12-07 23:17           ` [PATCH v2] " Cong Ding
  2012-12-07 23:28             ` H. Peter Anvin
@ 2012-12-08  0:04             ` tip-bot for Cong Ding
  2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
  2 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Cong Ding @ 2012-12-08  0:04 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, dinggnu, tglx, hpa

Commit-ID:  8d7474a0ddc619f4c9ebfc19264e3ef0906a7e6a
Gitweb:     http://git.kernel.org/tip/8d7474a0ddc619f4c9ebfc19264e3ef0906a7e6a
Author:     Cong Ding <dinggnu@gmail.com>
AuthorDate: Fri, 7 Dec 2012 23:14:32 +0000
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 7 Dec 2012 15:31:49 -0800

x86: Remove duplicate "const" in gen-insn-attr-x86.awk

Fix the following sparse warnings due to duplicate "const" keywords:
arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const

for the variables inat_escape_tables, inat_group_tables, and
inat_avx_tables in the code generated by gen-insn-attr-x86.awk.

Signed-off-by: Cong Ding <dinggnu@gmail.com>
Link: http://lkml.kernel.org/r/20121207231729.GC6179@gmail.com
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..987c7b2 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -374,7 +374,7 @@ END {
 	print "};\n"
 	# print AVX opcode map's array
 	print "/* AVX opcode map array */"
-	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < gaid; i++)
 		for (j = 0; j < max_lprefix; j++)

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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-07 23:17           ` [PATCH v2] " Cong Ding
  2012-12-07 23:28             ` H. Peter Anvin
  2012-12-08  0:04             ` [tip:x86/cleanups] x86: Remove duplicate "const" in gen-insn-attr-x86.awk tip-bot for Cong Ding
@ 2012-12-09  5:24             ` Masami Hiramatsu
  2012-12-09  8:21               ` [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk Cong Ding
                                 ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2012-12-09  5:24 UTC (permalink / raw)
  To: Cong Ding
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

(2012/12/08 8:17), Cong Ding wrote:
>>>>>> Patch description please?
>>>>> there are 2 consts in the definition of one variable
>>>>>
>>>>
>>>> Please put in an actual patch description.  The first line (subject
>>>> line) is a title; the patch should make sense without it.
>>> sorry for that. so like this is fine?
>>>
>>
>> Well, except that typically you should explain which variable it is.
>> Yes, it is obvious if you look at the patch, but you're making the
>> reader spend a few more moments than necessary.
>>
>> Also, you should explain what the harm is -- if it breaks anything
>> or is just a cosmetic issue.
> sorry again for lacking of experience...
> and I missed another same error, so send version 2.

Ah, sorry for my mistake. I would like to make both the value
pointed by the pointers and the pointers itself read-only.
Thus the right way of the patch should be;

-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \

Cong, could you update your patch? then I can Ack that.

Thank you,

> 
> - cong
> ---
> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00 2001
> From: Cong Ding <dinggnu@gmail.com>
> Date: Fri, 7 Dec 2012 23:14:32 +0000
> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
> 
> fix the following sparse warning:
> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
> 
> for variable inat_escape_tables, inat_group_tables, and inat_avx_tables
> 
> Signed-off-by: Cong Ding <dinggnu@gmail.com>
> ---
>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
> index ddcf39b..987c7b2 100644
> --- a/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -356,7 +356,7 @@ END {
>  		exit 1
>  	# print escape opcode map's array
>  	print "/* Escape opcode map array */"
> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
>  	      "[INAT_LSTPFX_MAX + 1] = {"
>  	for (i = 0; i < geid; i++)
>  		for (j = 0; j < max_lprefix; j++)
> @@ -365,7 +365,7 @@ END {
>  	print "};\n"
>  	# print group opcode map's array
>  	print "/* Group opcode map array */"
> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>  	      "[INAT_LSTPFX_MAX + 1] = {"
>  	for (i = 0; i < ggid; i++)
>  		for (j = 0; j < max_lprefix; j++)
> @@ -374,7 +374,7 @@ END {
>  	print "};\n"
>  	# print AVX opcode map's array
>  	print "/* AVX opcode map array */"
> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX + 1]"\
> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>  	      "[INAT_LSTPFX_MAX + 1] = {"
>  	for (i = 0; i < gaid; i++)
>  		for (j = 0; j < max_lprefix; j++)
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk
  2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
@ 2012-12-09  8:21               ` Cong Ding
  2012-12-10 21:17                 ` [tip:x86/cleanups] x86: Fix " tip-bot for Cong Ding
  2012-12-09  8:27               ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Cong Ding
  2012-12-09 15:50               ` H. Peter Anvin
  2 siblings, 1 reply; 20+ messages in thread
From: Cong Ding @ 2012-12-09  8:21 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

>From 9523e1de9d2771dc66a5b645651fc9f4745eb685 Mon Sep 17 00:00:00 2001
From: Cong Ding <dinggnu@gmail.com>
Date: Sun, 9 Dec 2012 08:06:20 +0000
Subject: [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk

x86: fix the error of using "const" in gen-insn-attr-x86.awk

The original version code causes following sparse warnings:
arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const

for the variables inat_escape_tables, inat_group_tables, and inat_avx_tables
in the code generated by gen-insn-attr-x86.awk.

The author Masami Hiramutsu says here is to make both the value pointed by the
pointers and the pointers itself read-only, so we move the "const" to be after
the "*".

Signed-off-by: Cong Ding <dinggnu@gmail.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..e6773dc 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t * const inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -374,7 +374,7 @@ END {
 	print "};\n"
 	# print AVX opcode map's array
 	print "/* AVX opcode map array */"
-	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+	print "const insn_attr_t * const inat_avx_tables[X86_VEX_M_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < gaid; i++)
 		for (j = 0; j < max_lprefix; j++)
-- 
1.7.4.5


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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
  2012-12-09  8:21               ` [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk Cong Ding
@ 2012-12-09  8:27               ` Cong Ding
  2012-12-09 15:50               ` H. Peter Anvin
  2 siblings, 0 replies; 20+ messages in thread
From: Cong Ding @ 2012-12-09  8:27 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

On Sun, Dec 09, 2012 at 02:24:55PM +0900, Masami Hiramatsu wrote:
> (2012/12/08 8:17), Cong Ding wrote:
> >>>>>> Patch description please?
> >>>>> there are 2 consts in the definition of one variable
> >>>>>
> >>>>
> >>>> Please put in an actual patch description.  The first line (subject
> >>>> line) is a title; the patch should make sense without it.
> >>> sorry for that. so like this is fine?
> >>>
> >>
> >> Well, except that typically you should explain which variable it is.
> >> Yes, it is obvious if you look at the patch, but you're making the
> >> reader spend a few more moments than necessary.
> >>
> >> Also, you should explain what the harm is -- if it breaks anything
> >> or is just a cosmetic issue.
> > sorry again for lacking of experience...
> > and I missed another same error, so send version 2.
> 
> Ah, sorry for my mistake. I would like to make both the value
> pointed by the pointers and the pointers itself read-only.
> Thus the right way of the patch should be;
> 
> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> 
> Cong, could you update your patch? then I can Ack that.
Hi Masami, Thank you for the note.

Hi Peter, I have updated and sent version 3, could you please help me update
it?

Thanks,
- cong

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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
  2012-12-09  8:21               ` [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk Cong Ding
  2012-12-09  8:27               ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Cong Ding
@ 2012-12-09 15:50               ` H. Peter Anvin
  2012-12-10  1:00                 ` Masami Hiramatsu
  2 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-09 15:50 UTC (permalink / raw)
  To: Masami Hiramatsu, Cong Ding
  Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel, yrl.pp-manager.tt

No, that would really be wrong - changing the type.

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

>(2012/12/08 8:17), Cong Ding wrote:
>>>>>>> Patch description please?
>>>>>> there are 2 consts in the definition of one variable
>>>>>>
>>>>>
>>>>> Please put in an actual patch description.  The first line
>(subject
>>>>> line) is a title; the patch should make sense without it.
>>>> sorry for that. so like this is fine?
>>>>
>>>
>>> Well, except that typically you should explain which variable it is.
>>> Yes, it is obvious if you look at the patch, but you're making the
>>> reader spend a few more moments than necessary.
>>>
>>> Also, you should explain what the harm is -- if it breaks anything
>>> or is just a cosmetic issue.
>> sorry again for lacking of experience...
>> and I missed another same error, so send version 2.
>
>Ah, sorry for my mistake. I would like to make both the value
>pointed by the pointers and the pointers itself read-only.
>Thus the right way of the patch should be;
>
>-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
>\
>+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>1]" \
>
>Cong, could you update your patch? then I can Ack that.
>
>Thank you,
>
>> 
>> - cong
>> ---
>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>2001
>> From: Cong Ding <dinggnu@gmail.com>
>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>duplicate const
>> 
>> fix the following sparse warning:
>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>> 
>> for variable inat_escape_tables, inat_group_tables, and
>inat_avx_tables
>> 
>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>> ---
>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>b/arch/x86/tools/gen-insn-attr-x86.awk
>> index ddcf39b..987c7b2 100644
>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>> @@ -356,7 +356,7 @@ END {
>>  		exit 1
>>  	# print escape opcode map's array
>>  	print "/* Escape opcode map array */"
>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>1]" \
>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>  	for (i = 0; i < geid; i++)
>>  		for (j = 0; j < max_lprefix; j++)
>> @@ -365,7 +365,7 @@ END {
>>  	print "};\n"
>>  	# print group opcode map's array
>>  	print "/* Group opcode map array */"
>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX +
>1]"\
>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>  	for (i = 0; i < ggid; i++)
>>  		for (j = 0; j < max_lprefix; j++)
>> @@ -374,7 +374,7 @@ END {
>>  	print "};\n"
>>  	# print AVX opcode map's array
>>  	print "/* AVX opcode map array */"
>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>1]"\
>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>  	for (i = 0; i < gaid; i++)
>>  		for (j = 0; j < max_lprefix; j++)
>> 

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-09 15:50               ` H. Peter Anvin
@ 2012-12-10  1:00                 ` Masami Hiramatsu
  2012-12-10  1:03                   ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Masami Hiramatsu @ 2012-12-10  1:00 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

(2012/12/10 0:50), H. Peter Anvin wrote:
> No, that would really be wrong - changing the type.

What would be wrong? IMHO, this is just a fix to add a fool
proof 'const' to array instance itself.
...Or, am I missed anything?

Thank you,

> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>> Patch description please?
>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>
>>>>>>
>>>>>> Please put in an actual patch description.  The first line
>> (subject
>>>>>> line) is a title; the patch should make sense without it.
>>>>> sorry for that. so like this is fine?
>>>>>
>>>>
>>>> Well, except that typically you should explain which variable it is.
>>>> Yes, it is obvious if you look at the patch, but you're making the
>>>> reader spend a few more moments than necessary.
>>>>
>>>> Also, you should explain what the harm is -- if it breaks anything
>>>> or is just a cosmetic issue.
>>> sorry again for lacking of experience...
>>> and I missed another same error, so send version 2.
>>
>> Ah, sorry for my mistake. I would like to make both the value
>> pointed by the pointers and the pointers itself read-only.
>> Thus the right way of the patch should be;
>>
>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
>> \
>> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>> 1]" \
>>
>> Cong, could you update your patch? then I can Ack that.
>>
>> Thank you,
>>
>>>
>>> - cong
>>> ---
>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>> 2001
>>> From: Cong Ding <dinggnu@gmail.com>
>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>> duplicate const
>>>
>>> fix the following sparse warning:
>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>
>>> for variable inat_escape_tables, inat_group_tables, and
>> inat_avx_tables
>>>
>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>> ---
>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>> index ddcf39b..987c7b2 100644
>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>> @@ -356,7 +356,7 @@ END {
>>>  		exit 1
>>>  	# print escape opcode map's array
>>>  	print "/* Escape opcode map array */"
>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>> 1]" \
>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>  	for (i = 0; i < geid; i++)
>>>  		for (j = 0; j < max_lprefix; j++)
>>> @@ -365,7 +365,7 @@ END {
>>>  	print "};\n"
>>>  	# print group opcode map's array
>>>  	print "/* Group opcode map array */"
>>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX +
>> 1]"\
>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>  	for (i = 0; i < ggid; i++)
>>>  		for (j = 0; j < max_lprefix; j++)
>>> @@ -374,7 +374,7 @@ END {
>>>  	print "};\n"
>>>  	# print AVX opcode map's array
>>>  	print "/* AVX opcode map array */"
>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>> 1]"\
>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>  	for (i = 0; i < gaid; i++)
>>>  		for (j = 0; j < max_lprefix; j++)
>>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-10  1:00                 ` Masami Hiramatsu
@ 2012-12-10  1:03                   ` H. Peter Anvin
  2012-12-10  1:27                     ` Masami Hiramatsu
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-10  1:03 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

Yes, if you add a * it becomes an array of pointers.

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

>(2012/12/10 0:50), H. Peter Anvin wrote:
>> No, that would really be wrong - changing the type.
>
>What would be wrong? IMHO, this is just a fix to add a fool
>proof 'const' to array instance itself.
>...Or, am I missed anything?
>
>Thank you,
>
>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>> 
>>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>>> Patch description please?
>>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>>
>>>>>>>
>>>>>>> Please put in an actual patch description.  The first line
>>> (subject
>>>>>>> line) is a title; the patch should make sense without it.
>>>>>> sorry for that. so like this is fine?
>>>>>>
>>>>>
>>>>> Well, except that typically you should explain which variable it
>is.
>>>>> Yes, it is obvious if you look at the patch, but you're making the
>>>>> reader spend a few more moments than necessary.
>>>>>
>>>>> Also, you should explain what the harm is -- if it breaks anything
>>>>> or is just a cosmetic issue.
>>>> sorry again for lacking of experience...
>>>> and I missed another same error, so send version 2.
>>>
>>> Ah, sorry for my mistake. I would like to make both the value
>>> pointed by the pointers and the pointers itself read-only.
>>> Thus the right way of the patch should be;
>>>
>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>1]"
>>> \
>>> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>>> 1]" \
>>>
>>> Cong, could you update your patch? then I can Ack that.
>>>
>>> Thank you,
>>>
>>>>
>>>> - cong
>>>> ---
>>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>>> 2001
>>>> From: Cong Ding <dinggnu@gmail.com>
>>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>>> duplicate const
>>>>
>>>> fix the following sparse warning:
>>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>>
>>>> for variable inat_escape_tables, inat_group_tables, and
>>> inat_avx_tables
>>>>
>>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>>> ---
>>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>>> index ddcf39b..987c7b2 100644
>>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>>> @@ -356,7 +356,7 @@ END {
>>>>  		exit 1
>>>>  	# print escape opcode map's array
>>>>  	print "/* Escape opcode map array */"
>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>>> 1]" \
>>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>  	for (i = 0; i < geid; i++)
>>>>  		for (j = 0; j < max_lprefix; j++)
>>>> @@ -365,7 +365,7 @@ END {
>>>>  	print "};\n"
>>>>  	# print group opcode map's array
>>>>  	print "/* Group opcode map array */"
>>>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX +
>>> 1]"\
>>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>  	for (i = 0; i < ggid; i++)
>>>>  		for (j = 0; j < max_lprefix; j++)
>>>> @@ -374,7 +374,7 @@ END {
>>>>  	print "};\n"
>>>>  	# print AVX opcode map's array
>>>>  	print "/* AVX opcode map array */"
>>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>>> 1]"\
>>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>  	for (i = 0; i < gaid; i++)
>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>
>> 

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-10  1:03                   ` H. Peter Anvin
@ 2012-12-10  1:27                     ` Masami Hiramatsu
  2012-12-10  1:34                       ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Masami Hiramatsu @ 2012-12-10  1:27 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

(2012/12/10 10:03), H. Peter Anvin wrote:
> Yes, if you add a * it becomes an array of pointers.

Right, I would like to make each pointer in the array read-only.

And, of course, the data itself which pointed by the pointer
is already protected.
You can see this in (builddir)/arch/x86/lib/inat_table.c
----
/* Table: 2-byte opcode (0x0f) */
const insn_attr_t inat_escape_table_1[INAT_OPCODE_TABLE_SIZE] = {
[...]
/* Escape opcode map array */
const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1][INAT_LSTPFX_MAX +
 1] = {
        [1][0] = inat_escape_table_1,
----

So I think Cong's v3 is good :)

Thank you,

> 
> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> (2012/12/10 0:50), H. Peter Anvin wrote:
>>> No, that would really be wrong - changing the type.
>>
>> What would be wrong? IMHO, this is just a fix to add a fool
>> proof 'const' to array instance itself.
>> ...Or, am I missed anything?
>>
>> Thank you,
>>
>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>
>>>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>>>> Patch description please?
>>>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>>>
>>>>>>>>
>>>>>>>> Please put in an actual patch description.  The first line
>>>> (subject
>>>>>>>> line) is a title; the patch should make sense without it.
>>>>>>> sorry for that. so like this is fine?
>>>>>>>
>>>>>>
>>>>>> Well, except that typically you should explain which variable it
>> is.
>>>>>> Yes, it is obvious if you look at the patch, but you're making the
>>>>>> reader spend a few more moments than necessary.
>>>>>>
>>>>>> Also, you should explain what the harm is -- if it breaks anything
>>>>>> or is just a cosmetic issue.
>>>>> sorry again for lacking of experience...
>>>>> and I missed another same error, so send version 2.
>>>>
>>>> Ah, sorry for my mistake. I would like to make both the value
>>>> pointed by the pointers and the pointers itself read-only.
>>>> Thus the right way of the patch should be;
>>>>
>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>> 1]"
>>>> \
>>>> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>>>> 1]" \
>>>>
>>>> Cong, could you update your patch? then I can Ack that.
>>>>
>>>> Thank you,
>>>>
>>>>>
>>>>> - cong
>>>>> ---
>>>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>>>> 2001
>>>>> From: Cong Ding <dinggnu@gmail.com>
>>>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>>>> duplicate const
>>>>>
>>>>> fix the following sparse warning:
>>>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>>>
>>>>> for variable inat_escape_tables, inat_group_tables, and
>>>> inat_avx_tables
>>>>>
>>>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>>>> ---
>>>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>>>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>> index ddcf39b..987c7b2 100644
>>>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>> @@ -356,7 +356,7 @@ END {
>>>>>  		exit 1
>>>>>  	# print escape opcode map's array
>>>>>  	print "/* Escape opcode map array */"
>>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +
>>>> 1]" \
>>>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>  	for (i = 0; i < geid; i++)
>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>> @@ -365,7 +365,7 @@ END {
>>>>>  	print "};\n"
>>>>>  	# print group opcode map's array
>>>>>  	print "/* Group opcode map array */"
>>>>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX +
>>>> 1]"\
>>>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>  	for (i = 0; i < ggid; i++)
>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>> @@ -374,7 +374,7 @@ END {
>>>>>  	print "};\n"
>>>>>  	# print AVX opcode map's array
>>>>>  	print "/* AVX opcode map array */"
>>>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>>>> 1]"\
>>>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>  	for (i = 0; i < gaid; i++)
>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>
>>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-10  1:27                     ` Masami Hiramatsu
@ 2012-12-10  1:34                       ` H. Peter Anvin
  2012-12-10  1:50                         ` Masami Hiramatsu
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-10  1:34 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

You're changing the array from an array of insn_attr_t to pointers to insn_attr_t?

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

>(2012/12/10 10:03), H. Peter Anvin wrote:
>> Yes, if you add a * it becomes an array of pointers.
>
>Right, I would like to make each pointer in the array read-only.
>
>And, of course, the data itself which pointed by the pointer
>is already protected.
>You can see this in (builddir)/arch/x86/lib/inat_table.c
>----
>/* Table: 2-byte opcode (0x0f) */
>const insn_attr_t inat_escape_table_1[INAT_OPCODE_TABLE_SIZE] = {
>[...]
>/* Escape opcode map array */
>const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>1][INAT_LSTPFX_MAX +
> 1] = {
>        [1][0] = inat_escape_table_1,
>----
>
>So I think Cong's v3 is good :)
>
>Thank you,
>
>> 
>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>> 
>>> (2012/12/10 0:50), H. Peter Anvin wrote:
>>>> No, that would really be wrong - changing the type.
>>>
>>> What would be wrong? IMHO, this is just a fix to add a fool
>>> proof 'const' to array instance itself.
>>> ...Or, am I missed anything?
>>>
>>> Thank you,
>>>
>>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>>
>>>>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>>>>> Patch description please?
>>>>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please put in an actual patch description.  The first line
>>>>> (subject
>>>>>>>>> line) is a title; the patch should make sense without it.
>>>>>>>> sorry for that. so like this is fine?
>>>>>>>>
>>>>>>>
>>>>>>> Well, except that typically you should explain which variable it
>>> is.
>>>>>>> Yes, it is obvious if you look at the patch, but you're making
>the
>>>>>>> reader spend a few more moments than necessary.
>>>>>>>
>>>>>>> Also, you should explain what the harm is -- if it breaks
>anything
>>>>>>> or is just a cosmetic issue.
>>>>>> sorry again for lacking of experience...
>>>>>> and I missed another same error, so send version 2.
>>>>>
>>>>> Ah, sorry for my mistake. I would like to make both the value
>>>>> pointed by the pointers and the pointers itself read-only.
>>>>> Thus the right way of the patch should be;
>>>>>
>>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX
>+
>>> 1]"
>>>>> \
>>>>> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX
>+
>>>>> 1]" \
>>>>>
>>>>> Cong, could you update your patch? then I can Ack that.
>>>>>
>>>>> Thank you,
>>>>>
>>>>>>
>>>>>> - cong
>>>>>> ---
>>>>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>>>>> 2001
>>>>>> From: Cong Ding <dinggnu@gmail.com>
>>>>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>>>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>>>>> duplicate const
>>>>>>
>>>>>> fix the following sparse warning:
>>>>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>>>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>>>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>>>>
>>>>>> for variable inat_escape_tables, inat_group_tables, and
>>>>> inat_avx_tables
>>>>>>
>>>>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>>>>> ---
>>>>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>> index ddcf39b..987c7b2 100644
>>>>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>> @@ -356,7 +356,7 @@ END {
>>>>>>  		exit 1
>>>>>>  	# print escape opcode map's array
>>>>>>  	print "/* Escape opcode map array */"
>>>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX
>+
>>>>> 1]" \
>>>>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]"
>\
>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>  	for (i = 0; i < geid; i++)
>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>> @@ -365,7 +365,7 @@ END {
>>>>>>  	print "};\n"
>>>>>>  	# print group opcode map's array
>>>>>>  	print "/* Group opcode map array */"
>>>>>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX
>+
>>>>> 1]"\
>>>>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>  	for (i = 0; i < ggid; i++)
>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>> @@ -374,7 +374,7 @@ END {
>>>>>>  	print "};\n"
>>>>>>  	# print AVX opcode map's array
>>>>>>  	print "/* AVX opcode map array */"
>>>>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>>>>> 1]"\
>>>>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>  	for (i = 0; i < gaid; i++)
>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>
>>>>
>> 

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-10  1:34                       ` H. Peter Anvin
@ 2012-12-10  1:50                         ` Masami Hiramatsu
  2012-12-10  1:56                           ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Masami Hiramatsu @ 2012-12-10  1:50 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

(2012/12/10 10:34), H. Peter Anvin wrote:
> You're changing the array from an array of insn_attr_t to pointers to insn_attr_t?

No, please look at the code carefully,

-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
                                      ^^
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
                                ^^
Both are pointers of array.

I'd just change the position of const.

const insn_attr_t const * -> const insn_attr_t * const

Thank you,

> 
> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> (2012/12/10 10:03), H. Peter Anvin wrote:
>>> Yes, if you add a * it becomes an array of pointers.
>>
>> Right, I would like to make each pointer in the array read-only.
>>
>> And, of course, the data itself which pointed by the pointer
>> is already protected.
>> You can see this in (builddir)/arch/x86/lib/inat_table.c
>> ----
>> /* Table: 2-byte opcode (0x0f) */
>> const insn_attr_t inat_escape_table_1[INAT_OPCODE_TABLE_SIZE] = {
>> [...]
>> /* Escape opcode map array */
>> const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>> 1][INAT_LSTPFX_MAX +
>> 1] = {
>>        [1][0] = inat_escape_table_1,
>> ----
>>
>> So I think Cong's v3 is good :)
>>
>> Thank you,
>>
>>>
>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>
>>>> (2012/12/10 0:50), H. Peter Anvin wrote:
>>>>> No, that would really be wrong - changing the type.
>>>>
>>>> What would be wrong? IMHO, this is just a fix to add a fool
>>>> proof 'const' to array instance itself.
>>>> ...Or, am I missed anything?
>>>>
>>>> Thank you,
>>>>
>>>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>>>
>>>>>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>>>>>> Patch description please?
>>>>>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please put in an actual patch description.  The first line
>>>>>> (subject
>>>>>>>>>> line) is a title; the patch should make sense without it.
>>>>>>>>> sorry for that. so like this is fine?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Well, except that typically you should explain which variable it
>>>> is.
>>>>>>>> Yes, it is obvious if you look at the patch, but you're making
>> the
>>>>>>>> reader spend a few more moments than necessary.
>>>>>>>>
>>>>>>>> Also, you should explain what the harm is -- if it breaks
>> anything
>>>>>>>> or is just a cosmetic issue.
>>>>>>> sorry again for lacking of experience...
>>>>>>> and I missed another same error, so send version 2.
>>>>>>
>>>>>> Ah, sorry for my mistake. I would like to make both the value
>>>>>> pointed by the pointers and the pointers itself read-only.
>>>>>> Thus the right way of the patch should be;
>>>>>>
>>>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX
>> +
>>>> 1]"
>>>>>> \
>>>>>> +	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX
>> +
>>>>>> 1]" \
>>>>>>
>>>>>> Cong, could you update your patch? then I can Ack that.
>>>>>>
>>>>>> Thank you,
>>>>>>
>>>>>>>
>>>>>>> - cong
>>>>>>> ---
>>>>>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00
>>>>>> 2001
>>>>>>> From: Cong Ding <dinggnu@gmail.com>
>>>>>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>>>>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>>>>>> duplicate const
>>>>>>>
>>>>>>> fix the following sparse warning:
>>>>>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>>>>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>>>>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>>>>>
>>>>>>> for variable inat_escape_tables, inat_group_tables, and
>>>>>> inat_avx_tables
>>>>>>>
>>>>>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>>>>>> ---
>>>>>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>>>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>> index ddcf39b..987c7b2 100644
>>>>>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>> @@ -356,7 +356,7 @@ END {
>>>>>>>  		exit 1
>>>>>>>  	# print escape opcode map's array
>>>>>>>  	print "/* Escape opcode map array */"
>>>>>>> -	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX
>> +
>>>>>> 1]" \
>>>>>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]"
>> \
>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>  	for (i = 0; i < geid; i++)
>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>> @@ -365,7 +365,7 @@ END {
>>>>>>>  	print "};\n"
>>>>>>>  	# print group opcode map's array
>>>>>>>  	print "/* Group opcode map array */"
>>>>>>> -	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX
>> +
>>>>>> 1]"\
>>>>>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>  	for (i = 0; i < ggid; i++)
>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>> @@ -374,7 +374,7 @@ END {
>>>>>>>  	print "};\n"
>>>>>>>  	# print AVX opcode map's array
>>>>>>>  	print "/* AVX opcode map array */"
>>>>>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX +
>>>>>> 1]"\
>>>>>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>  	for (i = 0; i < gaid; i++)
>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>>
>>>>>
>>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
  2012-12-10  1:50                         ` Masami Hiramatsu
@ 2012-12-10  1:56                           ` H. Peter Anvin
  0 siblings, 0 replies; 20+ messages in thread
From: H. Peter Anvin @ 2012-12-10  1:56 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Cong Ding, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	yrl.pp-manager.tt

Sorry, you're right.  I blame the font on my phone.

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

>(2012/12/10 10:34), H. Peter Anvin wrote:
>> You're changing the array from an array of insn_attr_t to pointers to
>insn_attr_t?
>
>No, please look at the code carefully,
>
>-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
>\
>                                      ^^
>+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>1]" \
>                                ^^
>Both are pointers of array.
>
>I'd just change the position of const.
>
>const insn_attr_t const * -> const insn_attr_t * const
>
>Thank you,
>
>> 
>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>> 
>>> (2012/12/10 10:03), H. Peter Anvin wrote:
>>>> Yes, if you add a * it becomes an array of pointers.
>>>
>>> Right, I would like to make each pointer in the array read-only.
>>>
>>> And, of course, the data itself which pointed by the pointer
>>> is already protected.
>>> You can see this in (builddir)/arch/x86/lib/inat_table.c
>>> ----
>>> /* Table: 2-byte opcode (0x0f) */
>>> const insn_attr_t inat_escape_table_1[INAT_OPCODE_TABLE_SIZE] = {
>>> [...]
>>> /* Escape opcode map array */
>>> const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX +
>>> 1][INAT_LSTPFX_MAX +
>>> 1] = {
>>>        [1][0] = inat_escape_table_1,
>>> ----
>>>
>>> So I think Cong's v3 is good :)
>>>
>>> Thank you,
>>>
>>>>
>>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>>
>>>>> (2012/12/10 0:50), H. Peter Anvin wrote:
>>>>>> No, that would really be wrong - changing the type.
>>>>>
>>>>> What would be wrong? IMHO, this is just a fix to add a fool
>>>>> proof 'const' to array instance itself.
>>>>> ...Or, am I missed anything?
>>>>>
>>>>> Thank you,
>>>>>
>>>>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>>>>
>>>>>>> (2012/12/08 8:17), Cong Ding wrote:
>>>>>>>>>>>>> Patch description please?
>>>>>>>>>>>> there are 2 consts in the definition of one variable
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Please put in an actual patch description.  The first line
>>>>>>> (subject
>>>>>>>>>>> line) is a title; the patch should make sense without it.
>>>>>>>>>> sorry for that. so like this is fine?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Well, except that typically you should explain which variable
>it
>>>>> is.
>>>>>>>>> Yes, it is obvious if you look at the patch, but you're making
>>> the
>>>>>>>>> reader spend a few more moments than necessary.
>>>>>>>>>
>>>>>>>>> Also, you should explain what the harm is -- if it breaks
>>> anything
>>>>>>>>> or is just a cosmetic issue.
>>>>>>>> sorry again for lacking of experience...
>>>>>>>> and I missed another same error, so send version 2.
>>>>>>>
>>>>>>> Ah, sorry for my mistake. I would like to make both the value
>>>>>>> pointed by the pointers and the pointers itself read-only.
>>>>>>> Thus the right way of the patch should be;
>>>>>>>
>>>>>>> -	print "const insn_attr_t const
>*inat_escape_tables[INAT_ESC_MAX
>>> +
>>>>> 1]"
>>>>>>> \
>>>>>>> +	print "const insn_attr_t * const
>inat_escape_tables[INAT_ESC_MAX
>>> +
>>>>>>> 1]" \
>>>>>>>
>>>>>>> Cong, could you update your patch? then I can Ack that.
>>>>>>>
>>>>>>> Thank you,
>>>>>>>
>>>>>>>>
>>>>>>>> - cong
>>>>>>>> ---
>>>>>>>> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17
>00:00:00
>>>>>>> 2001
>>>>>>>> From: Cong Ding <dinggnu@gmail.com>
>>>>>>>> Date: Fri, 7 Dec 2012 23:14:32 +0000
>>>>>>>> Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove
>>>>>>> duplicate const
>>>>>>>>
>>>>>>>> fix the following sparse warning:
>>>>>>>> arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
>>>>>>>> arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
>>>>>>>> arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const
>>>>>>>>
>>>>>>>> for variable inat_escape_tables, inat_group_tables, and
>>>>>>> inat_avx_tables
>>>>>>>>
>>>>>>>> Signed-off-by: Cong Ding <dinggnu@gmail.com>
>>>>>>>> ---
>>>>>>>>  arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
>>>>>>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>> b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>>> index ddcf39b..987c7b2 100644
>>>>>>>> --- a/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>>> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
>>>>>>>> @@ -356,7 +356,7 @@ END {
>>>>>>>>  		exit 1
>>>>>>>>  	# print escape opcode map's array
>>>>>>>>  	print "/* Escape opcode map array */"
>>>>>>>> -	print "const insn_attr_t const
>*inat_escape_tables[INAT_ESC_MAX
>>> +
>>>>>>> 1]" \
>>>>>>>> +	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX +
>1]"
>>> \
>>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>>  	for (i = 0; i < geid; i++)
>>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>>> @@ -365,7 +365,7 @@ END {
>>>>>>>>  	print "};\n"
>>>>>>>>  	# print group opcode map's array
>>>>>>>>  	print "/* Group opcode map array */"
>>>>>>>> -	print "const insn_attr_t const
>*inat_group_tables[INAT_GRP_MAX
>>> +
>>>>>>> 1]"\
>>>>>>>> +	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX +
>1]"\
>>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>>  	for (i = 0; i < ggid; i++)
>>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>>> @@ -374,7 +374,7 @@ END {
>>>>>>>>  	print "};\n"
>>>>>>>>  	# print AVX opcode map's array
>>>>>>>>  	print "/* AVX opcode map array */"
>>>>>>>> -	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX
>+
>>>>>>> 1]"\
>>>>>>>> +	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX +
>1]"\
>>>>>>>>  	      "[INAT_LSTPFX_MAX + 1] = {"
>>>>>>>>  	for (i = 0; i < gaid; i++)
>>>>>>>>  		for (j = 0; j < max_lprefix; j++)
>>>>>>>>
>>>>>>
>>>>
>> 

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

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

* [tip:x86/cleanups] x86: Fix the error of using "const" in gen-insn-attr-x86.awk
  2012-12-09  8:21               ` [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk Cong Ding
@ 2012-12-10 21:17                 ` tip-bot for Cong Ding
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Cong Ding @ 2012-12-10 21:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, masami.hiramatsu.pt, dinggnu, tglx, hpa

Commit-ID:  28a793892296ca3367193c7a7de1714f80049fd0
Gitweb:     http://git.kernel.org/tip/28a793892296ca3367193c7a7de1714f80049fd0
Author:     Cong Ding <dinggnu@gmail.com>
AuthorDate: Sun, 9 Dec 2012 08:21:04 +0000
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Mon, 10 Dec 2012 10:31:24 -0800

x86: Fix the error of using "const" in gen-insn-attr-x86.awk

The original version code causes following sparse warnings:
arch/x86/lib/inat-tables.c:1080:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1095:25: warning: duplicate const
arch/x86/lib/inat-tables.c:1118:25: warning: duplicate const

for the variables inat_escape_tables, inat_group_tables, and inat_avx_tables
in the code generated by gen-insn-attr-x86.awk.

The author Masami Hiramutsu says here is to make both the value pointed by the
pointers and the pointers itself read-only, so we move the "const" to be after
the "*".

Signed-off-by: Cong Ding <dinggnu@gmail.com>
Link: http://lkml.kernel.org/r/20121209082103.GA9181@gmail.com
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/tools/gen-insn-attr-x86.awk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index ddcf39b..e6773dc 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -356,7 +356,7 @@ END {
 		exit 1
 	# print escape opcode map's array
 	print "/* Escape opcode map array */"
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]" \
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < geid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -365,7 +365,7 @@ END {
 	print "};\n"
 	# print group opcode map's array
 	print "/* Group opcode map array */"
-	print "const insn_attr_t const *inat_group_tables[INAT_GRP_MAX + 1]"\
+	print "const insn_attr_t * const inat_group_tables[INAT_GRP_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < ggid; i++)
 		for (j = 0; j < max_lprefix; j++)
@@ -374,7 +374,7 @@ END {
 	print "};\n"
 	# print AVX opcode map's array
 	print "/* AVX opcode map array */"
-	print "const insn_attr_t const *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+	print "const insn_attr_t * const inat_avx_tables[X86_VEX_M_MAX + 1]"\
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < gaid; i++)
 		for (j = 0; j < max_lprefix; j++)

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

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

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-07 22:42 [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Cong Ding
2012-12-07 22:45 ` H. Peter Anvin
2012-12-07 22:49   ` Cong Ding
2012-12-07 22:56     ` H. Peter Anvin
2012-12-07 23:03       ` Cong Ding
2012-12-07 23:06         ` H. Peter Anvin
2012-12-07 23:17           ` [PATCH v2] " Cong Ding
2012-12-07 23:28             ` H. Peter Anvin
2012-12-08  0:04             ` [tip:x86/cleanups] x86: Remove duplicate "const" in gen-insn-attr-x86.awk tip-bot for Cong Ding
2012-12-09  5:24             ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Masami Hiramatsu
2012-12-09  8:21               ` [PATCH v3] x86: fix the error of using "const" in gen-insn-attr-x86.awk Cong Ding
2012-12-10 21:17                 ` [tip:x86/cleanups] x86: Fix " tip-bot for Cong Ding
2012-12-09  8:27               ` [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const Cong Ding
2012-12-09 15:50               ` H. Peter Anvin
2012-12-10  1:00                 ` Masami Hiramatsu
2012-12-10  1:03                   ` H. Peter Anvin
2012-12-10  1:27                     ` Masami Hiramatsu
2012-12-10  1:34                       ` H. Peter Anvin
2012-12-10  1:50                         ` Masami Hiramatsu
2012-12-10  1:56                           ` H. Peter Anvin

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).