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

[Carl-Daniel Hailfinger]

Am 08.05.2014 23:33 schrieb Stefan Tauner:
> On Thu, 08 May 2014 23:24:22 +0200
> Carl-Daniel Hailfinger  wrote:
> > Am 08.05.2014 23:11 schrieb Stefan Tauner:
> > > On Thu, 08 May 2014 22:51:43 +0200
> > > Carl-Daniel Hailfinger  wrote:
> > > > Am 08.05.2014 18:56 schrieb Stefan Tauner:
> > > > > On Tue, 19 Nov 2013 20:35:57 +0100
> > > > > Stefan Reinauer  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).

Regards,
Carl-Daniel