Nico Huber has posted comments on this change. ( https://review.coreboot.org/20250 )
Change subject: fixup of "Kill doit()" ......................................................................
Patch Set 1: Code-Review+2
Re: the guards, then you'd need to build internal.o always (since Makefile doesnt build it if no CONFIG_INTERNAL), and include an useless piece of .bss in the binary. You dislike guards that much?
I wouldn't mind the wasted .bss space when it makes the code more readable. I wrongly assumed we'd always compile internal.c (guess why, it has CONFIG_INTERNAL guards in it, OMG). Since it's not, your patch looks suddenly good :-P
Oh, the handling of that flag seems all kinds of weird to me, in general.
My fault. I needed to expose it through the library interface. Don't know why I didn't realize that the flashctx is the wrong place for the flags back then. Probably because there was no other context...
I could go with not having it in the flashctx, which would make it an internal programmer parameter that flashrom_image_write happens to look at (but there's guards there for it anyways already). (Having it only in flashctx comes with the issue that the flashctx doesnt exist when initializing the programmer...)
The mid-term idea is to either make flashctx always available or add a programmer context (cf. `struct flash_programmer` in libflashrom.h).
Or it could be split into two parameters, but ... ehh, i'm not even sure if having the flashctx part makes sense without internal, etc.
Eh, no. It should just move (together with all other global variables) into a programmer context.
This is very confusing to me and I'm here to fix the build, not fix the architechture :P
Agreed.
-
Nico Huber (Code Review)