* [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@hitachi.com (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@hitachi.com >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
* [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
* 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@hitachi.com 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@hitachi.com 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@hitachi.com (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@hitachi.com 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@hitachi.com (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@hitachi.com 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@hitachi.com (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@hitachi.com 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
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).