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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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

2012-12-09 Thread Cong Ding
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

2012-12-09 Thread Cong Ding
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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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-09 Thread Masami Hiramatsu
(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

2012-12-09 Thread H. Peter Anvin
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 Thread Masami Hiramatsu
(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 Thread Masami Hiramatsu
(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

2012-12-07 Thread H. Peter Anvin

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

2012-12-07 Thread H. Peter Anvin

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/