[coreboot] [PATCH] flashrom patch easier board_pciid_enables parsing

[Peter Stuge]

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.
> 
> The pain I was referring to is in the loads of NULL/0 stuff. It can be
> eliminated with liberal application of grep.

I'm afraid this doesn't help me understand your point about making
code painful. :\


> The real question is why there are so many NULL/0 entries in the
> first place.

Because many boards have non-specific PCI IDs?


> 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.

What did you look up in flash.h? When I've poked at this table I just
banged in some PCI IDs, -m strings if the IDs weren't good enough for
an automatic match, the pretty name and the function that should be
run. The function was often reused from another, similar, board, thus
allowing me to do the change with only a single file open, not that I
consider that a very important metric anyway.


> > 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.

Maybe they just aren't serious enough?


> Well, that's not entirely true. I bothered to send a patch, but
> iirc it was never acked. I can resend...

What should I search for? I can probably find the thread. Would've
been real handy to have it in trac though.


..

> And that makes it easier for new contributors who just want to fix
> flashrom for their board.

I dare say they aren't exactly the common case. Of course I would
like it if they were, but for now they aren't. I don't believe this
bikeshed actually plays even a small part in their (lack of)
contributions anyway.


> The two-line layout was really painful. It was multiline (which you
> seem to dislike extremely)

That's why I think the fourteen-line layout with mostly noise is bad.
The 2-line layout has confused me too a few times, but it was nothing
an extra carriage return while editing didn't fix immediately.


..

> I consider anything to be an improvement over the old layout.

You might be able to sell me on a single line layout, but I actually
think the two-line layout is as good as it gets here.


//Peter