[flashrom] [PATCH] CID1130005: Array compared against 0

[Stefan Tauner]

On Fri, 09 May 2014 00:53:39 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 08.05.2014 23:33 schrieb Stefan Tauner:
> > On Thu, 08 May 2014 23:24:22 +0200
> > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> >
> >> Am 08.05.2014 23:11 schrieb Stefan Tauner:
> >>> On Thu, 08 May 2014 22:51:43 +0200
> >>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> >>>
> >>>> Am 08.05.2014 18:56 schrieb Stefan Tauner:
> >>>>> On Tue, 19 Nov 2013 20:35:57 +0100
> >>>>> Stefan Reinauer <stefan.reinauer at coreboot.org> wrote:
> >>>>>
> >>>>>>> CID1130005: Array compared against 0
> >>>>>>>
> >>>>>>> The address of an array is never NULL, so the comparison will always evaluate
> >>>>>>> the same way.
> >>>>>>> In selfcheck: Array compared against NULL pointer
> >>>>>>>
> >>>>>>> Since the array is defined unconditionally in C code the check does not really
> >>>>>>> make sense. It might make more sense to check whether there are entries in the
> >>>>>>> array, but that is not required on all platforms so far.
> >>>>> Thanks for reporting this. I have attached by approach to fix this.
> >>>>> IMHO it makes no sense to check the array outside its compilation unit.
> >>>>> That's just stupid. Instead I move the checks of the flashchips array
> >>>>> from flashrom.c to flashchips.c and remove all others that are futile
> >>>>> anyway.
> >>>> NACK. flashchips.c should only have data, not code.
> >>> Says who, and more importantly why? :)
> >> I say that.
> >> The file is called flashchips.c and not
> >> flashchip_definitions_and_functions.c. For example, we could split
> >> flashchips.c and add each group of chips to the associated driver file,
> >> e.g. spi25.c. If we indeed start introducing code to flashchips.c, we
> >> might as well kill the separation altogether. Besides that, right now
> >> that file is nicely grepable and can be processed easily with Unix
> >> command line tools for texts. I used that often in the past, and having
> >> to use head/tail in addition to grep just to weed out the non-data stuff
> >> is not exactly the kind of fun I'm looking for.
> >>
> >> I hope the reasons above are not totally unreasonable.
> > Not completely unreasonable but I don't buy them. Anyway, what about
> > exporting the size of the array to a global variable?
> 
> Excellent idea. That would actually allow us to check for nonzero size
> of all arrays, and it would also allow us to check if the last member is
> the zero-filled member. Both conditions together are a good safeguard
> against accidental array overrun (e.g. someone being accidentally too
> broad with some #if 0 or architecture-specific #ifdef).

Let's start with the flashchips array for now. New patch attached.

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-selfcheck-of-various-arrays.patch
Type: text/x-patch
Size: 4379 bytes
Desc: not available
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20140509/8ef1c8af/attachment.patch>