* [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 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 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 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).