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
Attachment:
signature.asc
Description: Digital signature
