Messages in this thread | | | Subject | Re: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const | From | "H. Peter Anvin" <> | Date | Sun, 09 Dec 2012 17:34:27 -0800 |
| |
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.
| |