On 04.07.2008 18:25, Peter Stuge wrote:
> On Fri, Jul 04, 2008 at 06:11:54PM +0200, Carl-Daniel Hailfinger wrote:
>   
>> +            (!isprint(*(const char *)(bios + size - *(walk - 1))) &&
>> +            (*(const char *)(bios + size - *(walk - 1)))) ||
>> +            (!isprint(*(const char *)(bios + size - *(walk - 2))) &&
>> +            (*(const char *)(bios + size - *(walk - 2))))) {
>>     
>
> NAK. I would like to fix this in a better way, or not at all. A four
> line long condition can simply not be the best way, even in the short
> term.
>   

Hm. I could make that a lot more readable. How about this? By the way,
the whole show_id function is and was completely broken when compiled
for 64bit.

Index: flashrom-tmp1/layout.c
===================================================================
--- flashrom-tmp1/layout.c      (Revision 3412)
+++ flashrom-tmp1/layout.c      (Arbeitskopie)
@@ -44,7 +44,9 @@
 
 int show_id(uint8_t *bios, int size, int force)
 {
+#warning This code is completely broken on 64bit
        unsigned int *walk;
+       unsigned int mb_part_offset, mb_vendor_offset;
 
        walk = (unsigned int *)(bios + size - 0x10);
        walk--;
@@ -63,12 +65,14 @@
         * are outside the image of if the start of ID strings are nonsensical
         * (nonprintable and not \0).
         */
+       mb_part_offset = *(walk - 1);
+       mb_vendor_offset = *(walk - 2);
        if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || *walk > size ||
-               *(walk - 1) > size || *(walk - 2) > size ||
-               (!isprint((const char *)(bios + size - *(walk - 1))) &&
-               ((const char *)(bios + size - *(walk - 1)))) ||
-               (!isprint((const char *)(bios + size - *(walk - 2))) &&
-               ((const char *)(bios + size - *(walk - 2))))) {
+               mb_part_offset > size || mb_vendor_offset > size ||
+               (!isprint(*(const char *)(bios + size - mb_part_offset)) &&
+               (*(const char *)(bios + size - mb_part_offset))) ||
+               (!isprint(*(const char *)(bios + size - mb_vendor_offset)) &&
+               (*(const char *)(bios + size - mb_vendor_offset)))) {
                printf("Flash image seems to be a legacy BIOS. Disabling 
checks.\n");
                mainboard_vendor = def_name;
                mainboard_part = def_name;


-- 
http://www.hailfinger.org/


-- 
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to