Attention is currently required from: Douglas Anderson. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50206 )
Change subject: linux_mtd: Switch fopen() to open() for MTD devices ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/50206/comment/5295152b_8fe1be95 PS2, Line 26: overhead / translation that we just don't need.
I mean, the OS exposes the open/close/read/write calls and so the stdio functions _must_ be implemen […]
I meant the "just don't need" it part. Sorry for the brevity.
It's hard to prove for either API if an implementation (always) results in the correct behavior. Hence, I'd prefer to only change things if there are known problems.
Thanks for disabling the buffering btw. It's quite possible that I suggested to use the higher level API in the upstream review. I must have assumed that libc is smarter or something, e.g. enables buffering based an the file type ._.
Patchset:
PS2:
IMHO if we are accessing a driver via device file, any interrupt should be considered as failure, not silently retrying...
The error has to be handled somewhere, though. We shouldn't leave the (library) user without a clue that it might be worth to try again. Among the reasons for a short item count, the manual pages read(2), read(3), mention signals. It seems hard to decide in advance if that should always be treated as an error. Unless there is some special device node semantic, of course.
File linux_mtd.c:
https://review.coreboot.org/c/flashrom/+/50206/comment/52ddcd97_c7a1f1e8 PS2, Line 206: if (read(dev_fileno, buf + i, step) != step) {
Huh, this is a good point and I didn't think about it. […]
It's probably not hard to get around. (Sorry for not replying earlier, I didn't expect this to get abandoned this fast.)
This should directly map to a read of the flash right? Then we could just re-try the current block (a few times?) if `>= 0 && != step`.
I couldn't find any documentation specific to the MTD device nodes, do you know any? I'm also not much experienced with Linux device nodes in general. I guess there could be exceptions to the read() behavior, and it's fair to assume Linux behaviour in this driver anyway.
-
Nico Huber (Code Review)