[coreboot] [PATCH] flashrom patch easier board_pciid_enables parsing

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Jan 15 04:16:38 CET 2009


On 15.01.2009 03:55, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> Making strange code painful to look at is good.
>>     
>
> ..to ensure that noone will ever want to look at it again, so that
> bugs won't be found? Makes no sense.
>
> I consider the opposite to be true, in particular code with strange
> logic should be easy on the eyes so that anyone who has even slight
> interest could actually review it and maybe even spot a bug.
>   

It is definitely easier on the eyes if you only have to look at two
places at the same time (array and board enable function) now instead of
three places (flash.h and array and board enable function) before.

The pain I was referring to is in the loads of NULL/0 stuff. It can be
eliminated with liberal application of grep. The real question is why
there are so many NULL/0 entries in the first place.


> If the code is painful $person will just laugh and go away, leaving
> bugs lingering..
>   

Actually, there are bugs lingering in related code, but nobody bothered
to fix it. Well, that's not entirely true. I bothered to send a patch,
but iirc it was never acked. I can resend...

Anyway, this change makes it easy to add a new board without having to
open two editor windows at the same time to find out the correct struct
ordering. And that makes it easier for new contributors who just want to
fix flashrom for their board.

The two-line layout was really painful. It was multiline (which you seem
to dislike extremely) and it required anybody changing it to have
flash.h open at the same time (which I think is more complicated than it
should be).
I consider anything to be an improvement over the old layout.

Regards,
Carl-Daniel

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





More information about the coreboot mailing list