New Defects reported by Coverity Scan for coreboot

Hi,

Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan.

1 new defect(s) introduced to coreboot found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)

** CID 1372941:  Control flow issues  (UNREACHABLE)
/src/northbridge/intel/i945/debug.c: 86 in dump_spd_registers()

________________________________________________________________________________________________________
*** CID 1372941:  Control flow issues  (UNREACHABLE)
/src/northbridge/intel/i945/debug.c: 86 in dump_spd_registers()
80     	device = DIMM0;
81     	while (device <= DIMM3) {
82     		int status = 0;
83     		int i;
84     		printk(BIOS_DEBUG, "\ndimm %02x", device);
85
> > > CID 1372941:  Control flow issues  (UNREACHABLE)
> > > Since the loop increment "i++;" is unreachable, the loop body will never execute more than once.

86     		for (i = 0; (i < 256); i++) {
87     			if ((i % 16) == 0)
88     				printk(BIOS_DEBUG, "\n%02x: ", i);
89     			status = smbus_read_byte(device, i);
90     			if (status < 0)
91     				printk(BIOS_DEBUG, "bad device: %02x\n", -status);

________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05...

To manage Coverity Scan email notifications for "[e-mail]", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05...

Can we please stop hiding semantic changes behind "found by checkpatch" (Re: New Defects reported by Coverity Scan for coreboot)

[Nico Huber]

Hi all,

don't get me wrong, I think style fixes are a good thing and appreciate
that people take the time for it. However, if we do massive changes to
comply with a tool like checkpatch.pl, we should make sure that single
commits either change style only, or really fix something which should
then be mentioned in the commit message.

With many changes in one commit, the former is hard to review but can
be verified by reproducibility of the binaries beyond the patch. We
have an option to exclude commit information in the resulting image:
`make BUILD_TIMELESS=1 what-jenkins-does` should do the trick.

Can we set up a Jenkins job for that? either manually triggered to help
the reviewer or maybe even automatically by a tag like `BUILD=binary-
stable`?.

Regards,
Nico

Can we please stop hiding semantic changes behind "found by checkpatch" (Re: New Defects reported by Coverity Scan for coreboot)

[Martin Roth]

Hi Nico,
  I think that's a great idea, and I'll see what I can set up.

We can search the commit message for "no functional changes" or
"binary-stable" and then run a build of the previous commit as well as
the current commit, then compare the checksums for each build to make
sure they match.

Martin

On Fri, Mar 24, 2017 at 9:58 AM, Nico Huber  wrote:
> Hi all,
>
> don't get me wrong, I think style fixes are a good thing and appreciate
> that people take the time for it. However, if we do massive changes to
> comply with a tool like checkpatch.pl, we should make sure that single
> commits either change style only, or really fix something which should
> then be mentioned in the commit message.
>
> With many changes in one commit, the former is hard to review but can
> be verified by reproducibility of the binaries beyond the patch. We
> have an option to exclude commit information in the resulting image:
> `make BUILD_TIMELESS=1 what-jenkins-does` should do the trick.
>
> Can we set up a Jenkins job for that? either manually triggered to help
> the reviewer or maybe even automatically by a tag like `BUILD=binary-
> stable`?.
>
> Regards,
> Nico