[coreboot] #102: flashrom: coreboot ROM image file identification heuristic is broken

[coreboot]

#102: flashrom: coreboot ROM image file identification heuristic is broken
-------------------------+--------------------------------------------------
    Reporter:  stuge     |         Owner:  somebody           
        Type:  defect    |        Status:  reopened           
    Priority:  major     |     Milestone:  flashrom v1.0      
   Component:  flashrom  |       Version:                     
  Resolution:            |      Keywords:  rom image heuristic
Dependencies:            |   Patchstatus:  there is no patch  
-------------------------+--------------------------------------------------

Comment(by hailfinger):

 Replying to [comment:6 anonymous]:
 > Is this what the author possibly meant?

 Not completely. The patch below is a pure warning fix, but it doesn't fix
 the implementation of the mechanism completely.

 >
 > {{{
 > Index: layout.c
 > ===================================================================
 > --- layout.c    (revision 3412)
 > +++ layout.c    (working copy)
 > @@ -65,9 +65,9 @@
 >          */
 >         if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || *walk > size ||
 >                 *(walk - 1) > size || *(walk - 2) > size ||
 > -               (!isprint((const char *)(bios + size - *(walk - 1))) &&
 > +               (!isprint(*(const char *)(bios + size - *(walk - 1))) &&
 >                 ((const char *)(bios + size - *(walk - 1)))) ||
 > -               (!isprint((const char *)(bios + size - *(walk - 2))) &&
 > +               (!isprint(*(const char *)(bios + size - *(walk - 2))) &&
 >                 ((const char *)(bios + size - *(walk - 2))))) {
 >                 printf("Flash image seems to be a legacy BIOS. Disabling
 checks.\n");
 >                 mainboard_vendor = def_name;
 > }}}
 >
 > Was this code ever tested or reviewed? It obviously never worked like
 that.

 It compiled cleanly without warnings on my machine (gcc 4.2.1), otherwise
 I wouldn't have committed it.

 And I would be very careful with claiming "it obviously never worked". If
 it was so obvious, why did you miss 50% of the pointer dereference bugs?
 Probably because you didn't read the code and only looked at compiler
 warnings.

-- 
Ticket URL: <http://tracker.coreboot.org/trac/coreboot/ticket/102#comment:7>
coreboot <http://www.coreboot.org/>