lkml.org 
[lkml]   [2012]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] arch/x86/tools/gen-insn-attr-x86.awk: remove duplicate const
    From
    Date
    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.


    \
     
     \ /
      Last update: 2012-12-10 03:21    [W:3.081 / U:0.524 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site