Re: [patch] edac: sb_edac: add sanity check to silence static checker

[Dan Carpenter <dan.carpenter@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]