Stefan Tauner posted comments on this change.

View Change

Patch set 1:-Code-Review

I actually looked in the codebase for other variable declarations
in for heads and didn't see any. Perhaps there are some that you
know of? In any case, if we don't want to support non-c99 then we
should make that explicit in the Makefile.

I suspect that this bug/error was only triggered by a recent patch,
which worked fine with Jenkins and a couple other versions of GCC.
This patch is just a quick fix, which will work no matter if we
force C99 compatibility or not.

Yes, you are right. My memory betrayed me and I did not check, sorry. The C90 declarations annoyed me from the beginning and I thought they were already removed but that's wrong (probably because - as I explained on IRC - forcing C99 on compilers is more problematic than one would think). So, yes, we should fix the code, but as a fixup! of the respective patch which is 9fe1fb7 (aka nicintel_eeprom: Support for I210 emulated EEprom). This also shows why a buildbot is a precious thing ;)

To view, visit change 21702. To unsubscribe, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31
Gerrit-Change-Number: 21702
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner@gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Comment-Date: Wed, 27 Sep 2017 07:50:13 +0000
Gerrit-HasComments: No