Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
Sorry, you're right. I blame the font on my phone. Masami Hiramatsu 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 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 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 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 Date: Fri, 7 Dec 2012 23:14:32 + 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 --- 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 >>> + >>>
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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 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 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 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 >>> Date: Fri, 7 Dec 2012 23:14:32 + >>> 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 >>> --- >>> 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
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
You're changing the array from an array of insn_attr_t to pointers to insn_attr_t? Masami Hiramatsu 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 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 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 >> Date: Fri, 7 Dec 2012 23:14:32 + >> 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 >> --- >> 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. -- To
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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 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 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 > Date: Fri, 7 Dec 2012 23:14:32 + > 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 > --- > 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...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
Yes, if you add a * it becomes an array of pointers. Masami Hiramatsu 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 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 Date: Fri, 7 Dec 2012 23:14:32 + 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 --- 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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 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 >>> Date: Fri, 7 Dec 2012 23:14:32 + >>> 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 >>> --- >>> 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...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
No, that would really be wrong - changing the type. 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. > >Thank you, > >> >> - cong >> --- >> From 6cf729b913287a6fc06325ca75ccf0efff9274e8 Mon Sep 17 00:00:00 >2001 >> From: Cong Ding >> Date: Fri, 7 Dec 2012 23:14:32 + >> 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 >> --- >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
No, that would really be wrong - changing the type. Masami Hiramatsu masami.hiramatsu...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
Yes, if you add a * it becomes an array of pointers. Masami Hiramatsu masami.hiramatsu...@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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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...@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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
You're changing the array from an array of insn_attr_t to pointers to insn_attr_t? Masami Hiramatsu masami.hiramatsu...@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...@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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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...@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...@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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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...@hitachi.com -- To unsubscribe from
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
Sorry, you're right. I blame the font on my phone. Masami Hiramatsu masami.hiramatsu...@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...@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...@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...@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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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. -- To unsubscribe from
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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 > Date: Fri, 7 Dec 2012 23:14:32 + > 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 > --- > 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...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
(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 ding...@gmail.com Date: Fri, 7 Dec 2012 23:14:32 + 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 ding...@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...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/