Hi Carl-Daniel,

On 04.02.22 21:59, Carl-Daniel Hailfinger wrote:
> I'm glad that there is an effort to participate in GSoC again and I hope
> it will be successful.
>
> Reading through the current GSoC ideas list, I would like to share a few
> thoughts.
>
> Unfortunately, some of these ideas are missing a bit of context or may
> cause needless code churn.

please don't see the project ideas as a detailed manual what a student
should do. Sure, we could provide more details, but I guess there should
be a balance. IMO, there should be enough room for a student to bring
forward their own ideas. Eventually, all details need to be discussed
before the implementation is started, of course.

> Missing context:
> - Design and implement new CLI based on libflashrom and extend the API
> as necessary. That would be the fourth or fifth user interface for
> flashrom. Now if you mean cli_classic.c should be rewritten to use
> libflashrom, that's something different. The idea doesn't specify more
> details, but please note that any CLI will have to be supported for the
> next 20 years and subsequent generations of developers may not be
> willing to do that.

I don't see a problem with additional CLIs. It depends on their purpose.
The idea is not to implement another flashrom frontend that supports all
features of cli_classic, but rather to allow simpler implementations
that are dedicated to specific use cases. For instance, a CLI that
copies data from / to a single flash region. With a stable API and
command-line syntax that would need close to 0 maintenance compared
to an all-in-one CLI.

> - Emulate hardware and filesystem in platform-independent way for unit
> tests. flashrom has builtin code for hardware emulation. Some more
> hardware is emulated quite well by qemu. There were various patches for
> qemu floating around to help testing flashrom hardware drivers, but
> someone (TM) would have to dig through our mailing list as well as
> others.

Well, you generally don't want to start an emulator within a unit test,
do you? :) IIRC, the idea is simply to implement whatever is necessary
to test a single function, for instance the `command()` function of a
SPI master. I think I once called that emulation if you want to do it
without the programmer hardware, but you could as well call it mocking.

> Some more guidance for students is definitely necessary.

Yes, of course. IMO, that can happen interactively.

> Code churn:
> - Refactor arguments parser. The rationale given is "Strings are
> complicated in C to work with. However, the idea of this is to
> deduplicate a lot of code.". I agree that strings in C need careful
> programming. That's also a reason why I doubt rewriting the code would
> be a good idea. This idea in particular seems to suggest that
> reinventing a full-blown argument parser building bespoke structures is
> a way to reduce complexity. In the past, we had proposals to store
> configuration data and even command line arguments in JSON or XML, then
> use a system library to parse it. The reasoning for the XML/JSON was
> that building your own parser was a bad idea. Someone else will come
> along and refactor things again if we merge a bespoke parser now. "Hey,
> thanks for your contribution, it will be replaced in the next GSoC" is
> not something we would want to advertise.

I don't think is going to explode. Nobody said something about drawing
non-standard dependencies in. We just want to avoid further code
duplication. Like most programmer code that takes arguments follows
the same pattern to process it, e.g. check if it's an integer, if
it's in range etc. One could also call it the natural evolution of
extract_programmer_param(). So basically, do what we always did, but
with a little more strategy.

> I do acknowledge that flashrom
> has gotten lots of refactoring, but I'm a bit afraid the changelogs
> suggest that in many cases "TEST=builds" seems to have been the only
> test that was performed. It's a good thing to have unit tests, but tests
> on real hardware should be performed if a driver has been refactored.
> After all, people rely on flashrom to not break their hardware.

I'm also concerned about the current, real-hardware test coverage.
However, personally, I draw a line between changes that affect internal
"BIOS" programming and those that don't. Internal programming must work
flawlessly. OTOH, external programmers are less scary, worst case (e.g.
downgrading flashrom is not an option) somebody needs to apply a hot-
fix. If there are big concerns about the internal programmers, we can
always limit refactoring to external ones. Actually, to avoid future
code duplication, we wouldn't have to touch existing drivers at all.
But it also has its benefits to have a uniform code base.

Nico