lkml.org 
[lkml]   [2011]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] edac: sb_edac: add sanity check to silence static checker
On Tue, Nov 01, 2011 at 10:53:18AM -0200, Mauro Carvalho Chehab wrote:
> >> --- a/drivers/edac/sb_edac.c
> >> +++ b/drivers/edac/sb_edac.c
> >> @@ -970,6 +970,12 @@ static int get_memory_error_data(struct mem_ctl_info *mci,
> >> break;
> >> prv = limit;
> >> }
> >> + if (n_tads == MAX_TAD) {
> >> + sprintf(msg, "Could not discover the memory channel");
> >
> > why the sprintf() ? can you not simply:
> > edac_mc_handle_ce_no_info(mci,"Could not discover the memory channel");
>
> Yes, please us the edac-specific call. I'm working on some patches that will
> provide an additional functionality to those edac report calls. So, using
> sprintf() won't do the right thing after applying those patches (likely for
> Kernel v3.3).
>

I did use the edac-specific call. Walter is saying that the
sprintf() is not needed because I could just pass the string
directly. That's true enough, but I'd prefer to leave it how I wrote
it so it matches everything else in this file. Also passing it
directly as edac_mc_handle_ce_no_info(mci,"Could not discover the memory channel");
makes the line too long for the 80 character limit so someone will
complain if I do that. This isn't a fast path so the sprintf()
doesn't hurt anything.

Also looking at it now, I suspect this patch might be a bug fix
instead of just to silence the warning. We have a similar check for
the MAX_SAD loop earlier to the check that I added here for MAX_TAD.

regards,
dan carpenter

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2011-11-01 15:01    [W:0.040 / U:9.992 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site