* [PATCH] i7core_edac: fix erroneous size of static array
@ 2011-10-13 18:24 Niklas Söderlund
2011-10-13 18:56 ` Borislav Petkov
2011-10-13 20:52 ` [PATCHv2] " Niklas Söderlund
0 siblings, 2 replies; 12+ messages in thread
From: Niklas Söderlund @ 2011-10-13 18:24 UTC (permalink / raw)
To: mchehab; +Cc: linux-edac, linux-kernel, Niklas Söderlund
Signed-off-by: Niklas Söderlund <niso@kth.se>
---
drivers/edac/i7core_edac.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 70ad892..e0f6096 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -441,7 +441,7 @@ static inline int numrow(u32 row)
static inline int numcol(u32 col)
{
- static int cols[8] = {
+ static int cols[4] = {
1 << 10, 1 << 11, 1 << 12, -EINVAL,
};
return cols[col & 0x3];
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 18:24 [PATCH] i7core_edac: fix erroneous size of static array Niklas Söderlund
@ 2011-10-13 18:56 ` Borislav Petkov
2011-10-13 19:03 ` Luck, Tony
2011-10-13 20:03 ` Niklas Söderlund
2011-10-13 20:52 ` [PATCHv2] " Niklas Söderlund
1 sibling, 2 replies; 12+ messages in thread
From: Borislav Petkov @ 2011-10-13 18:56 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: mchehab, linux-edac, linux-kernel
On Thu, Oct 13, 2011 at 02:24:54PM -0400, Niklas Söderlund wrote:
> Signed-off-by: Niklas Söderlund <niso@kth.se>
> ---
> drivers/edac/i7core_edac.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 70ad892..e0f6096 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
>
> static inline int numcol(u32 col)
> {
> - static int cols[8] = {
> + static int cols[4] = {
> 1 << 10, 1 << 11, 1 << 12, -EINVAL,
> };
Even better,
you could completely remove the number in the [] since the {}
initializer contains all array elements already. In this and the
remaining arrays in those small inline functions.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 18:56 ` Borislav Petkov
@ 2011-10-13 19:03 ` Luck, Tony
2011-10-13 19:14 ` Borislav Petkov
2011-10-13 20:03 ` Niklas Söderlund
1 sibling, 1 reply; 12+ messages in thread
From: Luck, Tony @ 2011-10-13 19:03 UTC (permalink / raw)
To: Borislav Petkov, Niklas Söderlund; +Cc: mchehab, linux-edac, linux-kernel
> - static int cols[8] = {
> + static int cols[4] = {
Why are these arrays "static"? Does that generate better code than
dynamic initialization?
-Tony
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 19:03 ` Luck, Tony
@ 2011-10-13 19:14 ` Borislav Petkov
2011-10-13 20:04 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2011-10-13 19:14 UTC (permalink / raw)
To: Luck, Tony; +Cc: Niklas Söderlund, mchehab, linux-edac, linux-kernel
On Thu, Oct 13, 2011 at 03:03:20PM -0400, Luck, Tony wrote:
> > - static int cols[8] = {
> > + static int cols[4] = {
>
> Why are these arrays "static"? Does that generate better code than
> dynamic initialization?
They look like lookup arrays for count of things based on bit settings
and as such they should be static because they don't ... change :). If
they were dynamic, then inlining them in every function callsite would
be not cool.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 18:56 ` Borislav Petkov
2011-10-13 19:03 ` Luck, Tony
@ 2011-10-13 20:03 ` Niklas Söderlund
2011-10-13 20:06 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2011-10-13 20:03 UTC (permalink / raw)
To: Borislav Petkov; +Cc: mchehab, linux-edac, linux-kernel
On 10/13/2011 08:56 PM, Borislav Petkov wrote:
> On Thu, Oct 13, 2011 at 02:24:54PM -0400, Niklas Söderlund wrote:
>> Signed-off-by: Niklas Söderlund <niso@kth.se>
>> ---
>> drivers/edac/i7core_edac.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
>> index 70ad892..e0f6096 100644
>> --- a/drivers/edac/i7core_edac.c
>> +++ b/drivers/edac/i7core_edac.c
>> @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
>>
>> static inline int numcol(u32 col)
>> {
>> - static int cols[8] = {
>> + static int cols[4] = {
>> 1 << 10, 1 << 11, 1 << 12, -EINVAL,
>> };
>
> Even better,
>
> you could completely remove the number in the [] since the {}
> initializer contains all array elements already. In this and the
> remaining arrays in those small inline functions.
>
Thanks for the feedback!
What would be the correct way of submitting an updated patch? Sending it
in this mail thread or create a new one?
// Niklas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 19:14 ` Borislav Petkov
@ 2011-10-13 20:04 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2011-10-13 20:04 UTC (permalink / raw)
To: Borislav Petkov
Cc: Luck, Tony, Niklas Söderlund, linux-edac, linux-kernel
Em 13-10-2011 16:14, Borislav Petkov escreveu:
> On Thu, Oct 13, 2011 at 03:03:20PM -0400, Luck, Tony wrote:
>>> - static int cols[8] = {
>>> + static int cols[4] = {
>>
>> Why are these arrays "static"? Does that generate better code than
>> dynamic initialization?
>
> They look like lookup arrays for count of things based on bit settings
> and as such they should be static because they don't ... change :). If
> they were dynamic, then inlining them in every function callsite would
> be not cool.
Btw, it probably makes sense to also declare as "const". In this specific case,
it is likely that gcc will discover that it is const anyway, and do some
optimization, but sometimes gcc don't do what I would expect ;)
Mauro.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] i7core_edac: fix erroneous size of static array
2011-10-13 20:03 ` Niklas Söderlund
@ 2011-10-13 20:06 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2011-10-13 20:06 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Borislav Petkov, linux-edac, linux-kernel
Em 13-10-2011 17:03, Niklas Söderlund escreveu:
> On 10/13/2011 08:56 PM, Borislav Petkov wrote:
>
>> On Thu, Oct 13, 2011 at 02:24:54PM -0400, Niklas Söderlund wrote:
>>> Signed-off-by: Niklas Söderlund <niso@kth.se>
>>> ---
>>> drivers/edac/i7core_edac.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
>>> index 70ad892..e0f6096 100644
>>> --- a/drivers/edac/i7core_edac.c
>>> +++ b/drivers/edac/i7core_edac.c
>>> @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
>>>
>>> static inline int numcol(u32 col)
>>> {
>>> - static int cols[8] = {
>>> + static int cols[4] = {
>>> 1 << 10, 1 << 11, 1 << 12, -EINVAL,
>>> };
>>
>> Even better,
>>
>> you could completely remove the number in the [] since the {}
>> initializer contains all array elements already. In this and the
>> remaining arrays in those small inline functions.
>>
>
>
> Thanks for the feedback!
>
> What would be the correct way of submitting an updated patch? Sending it
> in this mail thread or create a new one?
[PATCHv2] is the usual way. if you're using git, you may use the msgid of
the first message as a reference for the version 2.
Mauro.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2] i7core_edac: fix erroneous size of static array
2011-10-13 18:24 [PATCH] i7core_edac: fix erroneous size of static array Niklas Söderlund
2011-10-13 18:56 ` Borislav Petkov
@ 2011-10-13 20:52 ` Niklas Söderlund
2012-01-29 1:26 ` Niklas Söderlund
1 sibling, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2011-10-13 20:52 UTC (permalink / raw)
To: mchehab; +Cc: linux-edac, linux-kernel, Niklas Söderlund
Remove size form lookup arrays and mark them as const.
Signed-off-by: Niklas Söderlund <niso@kth.se>
---
drivers/edac/i7core_edac.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 70ad892..f50acfb 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -417,21 +417,21 @@ static inline int numdimms(u32 dimms)
static inline int numrank(u32 rank)
{
- static int ranks[4] = { 1, 2, 4, -EINVAL };
+ static const int ranks[] = { 1, 2, 4, -EINVAL };
return ranks[rank & 0x3];
}
static inline int numbank(u32 bank)
{
- static int banks[4] = { 4, 8, 16, -EINVAL };
+ static const int banks[] = { 4, 8, 16, -EINVAL };
return banks[bank & 0x3];
}
static inline int numrow(u32 row)
{
- static int rows[8] = {
+ static const int rows[] = {
1 << 12, 1 << 13, 1 << 14, 1 << 15,
1 << 16, -EINVAL, -EINVAL, -EINVAL,
};
@@ -441,7 +441,7 @@ static inline int numrow(u32 row)
static inline int numcol(u32 col)
{
- static int cols[8] = {
+ static const int cols[] = {
1 << 10, 1 << 11, 1 << 12, -EINVAL,
};
return cols[col & 0x3];
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv2] i7core_edac: fix erroneous size of static array
2011-10-13 20:52 ` [PATCHv2] " Niklas Söderlund
@ 2012-01-29 1:26 ` Niklas Söderlund
2012-01-29 2:01 ` Jesper Juhl
0 siblings, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2012-01-29 1:26 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: mchehab, linux-edac, linux-kernel
Hi,
Anyone have comments on this or are there no interest in this patch?
// Niklas
On 10/13/2011 10:52 PM, Niklas Söderlund wrote:
> Remove size form lookup arrays and mark them as const.
>
> Signed-off-by: Niklas Söderlund <niso@kth.se>
> ---
> drivers/edac/i7core_edac.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 70ad892..f50acfb 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> @@ -417,21 +417,21 @@ static inline int numdimms(u32 dimms)
>
> static inline int numrank(u32 rank)
> {
> - static int ranks[4] = { 1, 2, 4, -EINVAL };
> + static const int ranks[] = { 1, 2, 4, -EINVAL };
>
> return ranks[rank & 0x3];
> }
>
> static inline int numbank(u32 bank)
> {
> - static int banks[4] = { 4, 8, 16, -EINVAL };
> + static const int banks[] = { 4, 8, 16, -EINVAL };
>
> return banks[bank & 0x3];
> }
>
> static inline int numrow(u32 row)
> {
> - static int rows[8] = {
> + static const int rows[] = {
> 1 << 12, 1 << 13, 1 << 14, 1 << 15,
> 1 << 16, -EINVAL, -EINVAL, -EINVAL,
> };
> @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
>
> static inline int numcol(u32 col)
> {
> - static int cols[8] = {
> + static const int cols[] = {
> 1 << 10, 1 << 11, 1 << 12, -EINVAL,
> };
> return cols[col & 0x3];
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] i7core_edac: fix erroneous size of static array
2012-01-29 1:26 ` Niklas Söderlund
@ 2012-01-29 2:01 ` Jesper Juhl
2012-01-29 22:04 ` [PATCHv3] " Niklas Söderlund
0 siblings, 1 reply; 12+ messages in thread
From: Jesper Juhl @ 2012-01-29 2:01 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: mchehab, linux-edac, linux-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1794 bytes --]
On Sun, 29 Jan 2012, Niklas Söderlund wrote:
> Hi,
>
> Anyone have comments on this or are there no interest in this patch?
>
> // Niklas
>
> On 10/13/2011 10:52 PM, Niklas Söderlund wrote:
>
> > Remove size form lookup arrays and mark them as const.
> >
Besides the spelling mistake - s/form/from/ - here, it looks good to me
:-)
> > Signed-off-by: Niklas Söderlund <niso@kth.se>
> > ---
> > drivers/edac/i7core_edac.c | 8 ++++----
> > 1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> > index 70ad892..f50acfb 100644
> > --- a/drivers/edac/i7core_edac.c
> > +++ b/drivers/edac/i7core_edac.c
> > @@ -417,21 +417,21 @@ static inline int numdimms(u32 dimms)
> >
> > static inline int numrank(u32 rank)
> > {
> > - static int ranks[4] = { 1, 2, 4, -EINVAL };
> > + static const int ranks[] = { 1, 2, 4, -EINVAL };
> >
> > return ranks[rank & 0x3];
> > }
> >
> > static inline int numbank(u32 bank)
> > {
> > - static int banks[4] = { 4, 8, 16, -EINVAL };
> > + static const int banks[] = { 4, 8, 16, -EINVAL };
> >
> > return banks[bank & 0x3];
> > }
> >
> > static inline int numrow(u32 row)
> > {
> > - static int rows[8] = {
> > + static const int rows[] = {
> > 1 << 12, 1 << 13, 1 << 14, 1 << 15,
> > 1 << 16, -EINVAL, -EINVAL, -EINVAL,
> > };
> > @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
> >
> > static inline int numcol(u32 col)
> > {
> > - static int cols[8] = {
> > + static const int cols[] = {
> > 1 << 10, 1 << 11, 1 << 12, -EINVAL,
> > };
> > return cols[col & 0x3];
>
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3] i7core_edac: fix erroneous size of static array
2012-01-29 2:01 ` Jesper Juhl
@ 2012-01-29 22:04 ` Niklas Söderlund
2012-01-29 22:14 ` Jesper Juhl
0 siblings, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2012-01-29 22:04 UTC (permalink / raw)
To: jj; +Cc: mchehab, linux-edac, linux-kernel, Niklas Söderlund
Remove size from lookup arrays and mark them as const.
Signed-off-by: Niklas Söderlund <niso@kth.se>
---
drivers/edac/i7core_edac.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 70ad892..f50acfb 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -417,21 +417,21 @@ static inline int numdimms(u32 dimms)
static inline int numrank(u32 rank)
{
- static int ranks[4] = { 1, 2, 4, -EINVAL };
+ static const int ranks[] = { 1, 2, 4, -EINVAL };
return ranks[rank & 0x3];
}
static inline int numbank(u32 bank)
{
- static int banks[4] = { 4, 8, 16, -EINVAL };
+ static const int banks[] = { 4, 8, 16, -EINVAL };
return banks[bank & 0x3];
}
static inline int numrow(u32 row)
{
- static int rows[8] = {
+ static const int rows[] = {
1 << 12, 1 << 13, 1 << 14, 1 << 15,
1 << 16, -EINVAL, -EINVAL, -EINVAL,
};
@@ -441,7 +441,7 @@ static inline int numrow(u32 row)
static inline int numcol(u32 col)
{
- static int cols[8] = {
+ static const int cols[] = {
1 << 10, 1 << 11, 1 << 12, -EINVAL,
};
return cols[col & 0x3];
--
1.7.8.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv3] i7core_edac: fix erroneous size of static array
2012-01-29 22:04 ` [PATCHv3] " Niklas Söderlund
@ 2012-01-29 22:14 ` Jesper Juhl
0 siblings, 0 replies; 12+ messages in thread
From: Jesper Juhl @ 2012-01-29 22:14 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: mchehab, linux-edac, linux-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1519 bytes --]
On Sun, 29 Jan 2012, Niklas Söderlund wrote:
> Remove size from lookup arrays and mark them as const.
>
> Signed-off-by: Niklas Söderlund <niso@kth.se>
Reviewed-by: Jesper Juhl <jj@chaosbits.net>
> ---
> drivers/edac/i7core_edac.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 70ad892..f50acfb 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> @@ -417,21 +417,21 @@ static inline int numdimms(u32 dimms)
>
> static inline int numrank(u32 rank)
> {
> - static int ranks[4] = { 1, 2, 4, -EINVAL };
> + static const int ranks[] = { 1, 2, 4, -EINVAL };
>
> return ranks[rank & 0x3];
> }
>
> static inline int numbank(u32 bank)
> {
> - static int banks[4] = { 4, 8, 16, -EINVAL };
> + static const int banks[] = { 4, 8, 16, -EINVAL };
>
> return banks[bank & 0x3];
> }
>
> static inline int numrow(u32 row)
> {
> - static int rows[8] = {
> + static const int rows[] = {
> 1 << 12, 1 << 13, 1 << 14, 1 << 15,
> 1 << 16, -EINVAL, -EINVAL, -EINVAL,
> };
> @@ -441,7 +441,7 @@ static inline int numrow(u32 row)
>
> static inline int numcol(u32 col)
> {
> - static int cols[8] = {
> + static const int cols[] = {
> 1 << 10, 1 << 11, 1 << 12, -EINVAL,
> };
> return cols[col & 0x3];
>
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-01-29 22:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-13 18:24 [PATCH] i7core_edac: fix erroneous size of static array Niklas Söderlund
2011-10-13 18:56 ` Borislav Petkov
2011-10-13 19:03 ` Luck, Tony
2011-10-13 19:14 ` Borislav Petkov
2011-10-13 20:04 ` Mauro Carvalho Chehab
2011-10-13 20:03 ` Niklas Söderlund
2011-10-13 20:06 ` Mauro Carvalho Chehab
2011-10-13 20:52 ` [PATCHv2] " Niklas Söderlund
2012-01-29 1:26 ` Niklas Söderlund
2012-01-29 2:01 ` Jesper Juhl
2012-01-29 22:04 ` [PATCHv3] " Niklas Söderlund
2012-01-29 22:14 ` Jesper Juhl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).