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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 1 Nov 2011 16:59:45 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Mauro Carvalho Chehab <mchehab@...hat.com>
Cc:	wharms@....de, "Mark A. Grondona" <mgrondona@...l.gov>,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [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

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)