Skip to contentSkip to conversation

arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const

PATCH Superseded Cong Ding 3 participants 20 messages Dec 7, 2012 – Dec 10, 2012 x86

lore
Share & tools
lore

Cong Ding

author
#0 lore raw

PATCH 1/1 arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const

---
 arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +2 -2
index ddcf39b..d1d9cfa 100644
@@ -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++)
Signature · 1 line
1.7.4.5
#1 lore raw

Patch description please?

Quoted 30 lines · Cong Ding

---
arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--

1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +1 -1
index ddcf39b..d1d9cfa 100644
@@ -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++)
arch/x86/tools/gen-insn-attr-x86.awk +1 -1
@@ -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++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
author
#2 lore raw
Quoted 1 line · H. Peter Anvin

Patch description please?

there are 2 consts in the definition of one variable

- cong

Quoted 36 lines
Quoted 30 lines · Cong Ding

---
arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--

1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +1 -1
index ddcf39b..d1d9cfa 100644
@@ -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++)
arch/x86/tools/gen-insn-attr-x86.awk +1 -1
@@ -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++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#3 lore raw
Quoted 4 lines · Cong Ding
Quoted 1 line · H. Peter Anvin

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

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#4 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Cong Ding
Quoted 1 line · H. Peter Anvin

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

---

Quoted 1 line

From 1abfab824ed2dc0af6e283ea0b7a6c45541d4fd1 Mon Sep 17 00:00:00 2001

From: Cong Ding <[email protected]>
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.

---
 arch/x86/tools/gen-insn-attr-x86.awk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +2 -2
index ddcf39b..d1d9cfa 100644
@@ -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++)
Signature · 1 line
1.7.4.5
#5 lore raw
Quoted 11 lines · Cong Ding
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Cong Ding
Quoted 1 line · H. Peter Anvin

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

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#6 lore raw

PATCH v2 1/1 arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const

Quoted 13 lines · H. Peter Anvin
Quoted 11 lines · Cong Ding
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Cong Ding
Quoted 1 line · H. Peter Anvin

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?

Quoted 6 lines

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

Quoted 1 line

From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00 2001

From: Cong Ding <[email protected]>
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

---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +3 -3
index ddcf39b..987c7b2 100644
@@ -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++)
Signature · 1 line
1.7.4.5
#7 lore raw
Quoted 14 lines · Cong Ding
Quoted 13 lines · H. Peter Anvin
Quoted 11 lines · Cong Ding
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Cong Ding
Quoted 1 line · H. Peter Anvin

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?

Quoted 9 lines
Quoted 6 lines

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

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#8 lore raw

[tip:x86/cleanups] x86: Remove duplicate "const" in gen-insn-attr-x86.awk

In reply to Cong Ding
Commit-ID:  8d7474a0ddc619f4c9ebfc19264e3ef0906a7e6a
Gitweb:     http://git.kernel.org/tip/8d7474a0ddc619f4c9ebfc19264e3ef0906a7e6a
Author:     Cong Ding <[email protected]>
AuthorDate: Fri, 7 Dec 2012 23:14:32 +0000
Committer:  H. Peter Anvin <[email protected]>
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.

---
 arch/x86/tools/gen-insn-attr-x86.awk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +3 -3
index ddcf39b..987c7b2 100644
@@ -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++)
#9 lore raw
In reply to Cong Ding
Quoted 9 lines · Cong Ding
Quoted 9 lines
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

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?

Quoted 8 lines
Quoted 6 lines

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,

Quoted 52 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00 2001
From: Cong Ding <[email protected]>
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

---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +3 -3
index ddcf39b..987c7b2 100644
@@ -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++)
Signature · 4 lines
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
author
#10 lore raw

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

Quoted 1 line

From 9523e1de9d2771dc66a5b645651fc9f4745eb685 Mon Sep 17 00:00:00 2001

From: Cong Ding <[email protected]>
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 "*".

---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +3 -3
index ddcf39b..e6773dc 100644
@@ -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++)
Signature · 1 line
1.7.4.5
#11 lore raw

[tip:x86/cleanups] x86: Fix the error of using "const" in gen-insn-attr-x86.awk

Commit-ID:  28a793892296ca3367193c7a7de1714f80049fd0
Gitweb:     http://git.kernel.org/tip/28a793892296ca3367193c7a7de1714f80049fd0
Author:     Cong Ding <[email protected]>
AuthorDate: Sun, 9 Dec 2012 08:21:04 +0000
Committer:  H. Peter Anvin <[email protected]>
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 "*".

---
 arch/x86/tools/gen-insn-attr-x86.awk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/tools/gen-insn-attr-x86.awk +3 -3
index ddcf39b..e6773dc 100644
@@ -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++)
author
#12 lore raw
In reply to Masami Hiramatsu
Quoted 26 lines · Masami Hiramatsu
Quoted 9 lines · Cong Ding
Quoted 9 lines
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

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?

Quoted 8 lines
Quoted 6 lines

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;

(quoted diff) +1 -1
-	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]" \
Quoted 1 line

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

#13 lore raw
In reply to Masami Hiramatsu

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

Quoted 92 lines · Masami Hiramatsu
Quoted 5 lines · Cong Ding
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Please put in an actual patch description. The first line

(subject

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

line) is a title; the patch should make sense without it.

sorry for that. so like this is fine?

Quoted 8 lines
Quoted 6 lines

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"

\

(quoted diff) +1 -0
+	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,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#14 lore raw
Quoted 1 line · H. Peter Anvin

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,

Quoted 95 lines
Quoted 92 lines · Masami Hiramatsu
Quoted 5 lines · Cong Ding
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Please put in an actual patch description. The first line

(subject

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

line) is a title; the patch should make sense without it.

sorry for that. so like this is fine?

Quoted 8 lines
Quoted 6 lines

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"

\

(quoted diff) +1 -0
+	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,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 4 lines
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#15 lore raw

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

Quoted 106 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 7 lines
Quoted 5 lines · Masami Hiramatsu
Quoted 4 lines · Cong Ding
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Please put in an actual patch description. The first line

(subject

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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.

Quoted 8 lines

Review

Quoted 8 lines
Quoted 2 lines

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +

1]"

Quoted 68 lines
Quoted 67 lines

\

(quoted diff) +1 -0
+	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,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#16 lore raw
Quoted 1 line · H. Peter Anvin

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,

Quoted 110 lines
Quoted 106 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 7 lines
Quoted 5 lines · Masami Hiramatsu
Quoted 4 lines · Cong Ding
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Please put in an actual patch description. The first line

(subject

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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.

Quoted 8 lines

Review

Quoted 8 lines
Quoted 2 lines

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX +

1]"

Quoted 68 lines
Quoted 67 lines

\

(quoted diff) +1 -0
+	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,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 4 lines
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#17 lore raw

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

Quoted 140 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 20 lines
Quoted 17 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 8 lines
Quoted 6 lines · Masami Hiramatsu
Quoted 5 lines · Cong Ding
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Please put in an actual patch description. The first line

Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines

(subject

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Yes, it is obvious if you look at the patch, but you're making

the

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

reader spend a few more moments than necessary.

Also, you should explain what the harm is -- if it breaks

anything

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

Review

Quoted 9 lines
Quoted 3 lines
Quoted 1 line

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX

+

Quoted 3 lines
Quoted 3 lines

1]"

Quoted 2 lines

Review

Quoted 2 lines

\

(quoted diff) +1 -0
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX

+

Quoted 31 lines
Quoted 31 lines
Quoted 31 lines
Quoted 31 lines

1]" \

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

Thank you,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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

+

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

1]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 1 line
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]"

\

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 8 lines
arch/x86/tools/gen-insn-attr-x86.awk +0 -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

+

Quoted 18 lines
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines

1]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#18 lore raw
Quoted 1 line · H. Peter Anvin

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,

Quoted 144 lines
Quoted 140 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 20 lines
Quoted 17 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 8 lines
Quoted 6 lines · Masami Hiramatsu
Quoted 5 lines · Cong Ding
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Please put in an actual patch description. The first line

Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines

(subject

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Yes, it is obvious if you look at the patch, but you're making

the

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

reader spend a few more moments than necessary.

Also, you should explain what the harm is -- if it breaks

anything

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

Review

Quoted 9 lines
Quoted 3 lines
Quoted 1 line

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX

+

Quoted 3 lines
Quoted 3 lines

1]"

Quoted 2 lines

Review

Quoted 2 lines

\

(quoted diff) +1 -0
+	print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX

+

Quoted 31 lines
Quoted 31 lines
Quoted 31 lines
Quoted 31 lines

1]" \

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

Thank you,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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

+

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

1]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 1 line
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]"

\

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 8 lines
arch/x86/tools/gen-insn-attr-x86.awk +0 -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

+

Quoted 18 lines
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines

1]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 9 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -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]"\

Quoted 5 lines
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	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++)
Signature · 4 lines
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#19 lore raw

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

Quoted 174 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

(quoted diff) +0 -1
-	print "const insn_attr_t const *inat_escape_tables[INAT_ESC_MAX + 1]"
\
                                     ^^
(quoted diff) +1 -0
+	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,

Quoted 45 lines
Quoted 42 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 19 lines
Quoted 16 lines · Masami Hiramatsu
Quoted 1 line · H. Peter Anvin

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,

Quoted 7 lines
Quoted 5 lines · Masami Hiramatsu
Quoted 4 lines · Cong Ding
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

Patch description please?

there are 2 consts in the definition of one variable

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Please put in an actual patch description. The first line

(subject

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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

Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

is.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Yes, it is obvious if you look at the patch, but you're making

the

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

reader spend a few more moments than necessary.

Also, you should explain what the harm is -- if it breaks

anything

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

or is just a cosmetic issue.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Review

Quoted 8 lines
Quoted 2 lines

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;

(quoted diff) +0 -1
-	print "const insn_attr_t const

*inat_escape_tables[INAT_ESC_MAX

Quoted 4 lines
Quoted 4 lines

+

Quoted 3 lines
Quoted 3 lines

1]"

Quoted 2 lines

Review

Quoted 2 lines

\

(quoted diff) +1 -0
+	print "const insn_attr_t * const

inat_escape_tables[INAT_ESC_MAX

Quoted 11 lines
Quoted 11 lines

+

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

1]" \

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

Thank you,

Quoted 4 lines

- cong
---
From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17

00:00:00

Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines

2001

Quoted 3 lines

From: Cong Ding <[email protected]>
Date: Fri, 7 Dec 2012 23:14:32 +0000
Subject: [PATCH] arch/x86/tools/gen-insn-attr-x86.awk: remove

duplicate const

Quoted 7 lines

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

Review

Quoted 7 lines
---
 arch/x86/tools/gen-insn-attr-x86.awk |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/tools/gen-insn-attr-x86.awk

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 8 lines

index ddcf39b..987c7b2 100644

arch/x86/tools/gen-insn-attr-x86.awk +0 -1
@@ -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

Quoted 3 lines
Quoted 3 lines

+

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

1]" \

arch/x86/tools/gen-insn-attr-x86.awk END { · line 359

Quoted 1 line
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	print "const insn_attr_t *inat_escape_tables[INAT_ESC_MAX +

1]"

Quoted 9 lines
Quoted 9 lines

\

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 8 lines
arch/x86/tools/gen-insn-attr-x86.awk +0 -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

Quoted 3 lines
Quoted 3 lines

+

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

1]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 368

Quoted 1 line
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	print "const insn_attr_t *inat_group_tables[INAT_GRP_MAX +

1]"\

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 8 lines
arch/x86/tools/gen-insn-attr-x86.awk +0 -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

+

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

1]"\

arch/x86/tools/gen-insn-attr-x86.awk END { · line 377

Quoted 1 line
arch/x86/tools/gen-insn-attr-x86.awk +1 -0
+	print "const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX +

1]"\

Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
arch/x86/tools/gen-insn-attr-x86.awk +0 -0
 	      "[INAT_LSTPFX_MAX + 1] = {"
 	for (i = 0; i < gaid; i++)
 		for (j = 0; j < max_lprefix; j++)
Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.