Nico Huber has posted comments on this change. ( https://review.coreboot.org/25706 )
Change subject: linux_mtd: Initial import ......................................................................
Patch Set 5: Code-Review+2
(3 comments)
https://review.coreboot.org/#/c/25706/4/linux_mtd.c File linux_mtd.c:
https://review.coreboot.org/#/c/25706/4/linux_mtd.c@231 PS4, Line 231: perr(
I encountered a "Bad file descriptor" error when attempting to convert this to fwrite(), but will lo […]
Maybe there actually is an alignment requirement? In that case we'd have to retry partial writes. Doable, but maybe that gets too complicated.
This could also be a sign that fread() only works by chance. Maybe looking at an strace might give a clue.
If you end up keeping the mixed fread() / write() usage, you can also use fileno() on the stream object to avoid maintaining the additional `fd`.
https://review.coreboot.org/#/c/25706/5/linux_mtd.c File linux_mtd.c:
https://review.coreboot.org/#/c/25706/5/linux_mtd.c@208 PS5, Line 208: if (fread(buf + i, 1, step, dev_fp) != step) { `step, 1`, i.e. reading one big chunk, might be better optimized in libc.
https://review.coreboot.org/#/c/25706/5/linux_mtd.c@314 PS5, Line 314: , sizeof(buf) Nit, we know both `buf` and "nor" are zero-terminated. There is no need for str*n*cmp().
-
Nico Huber (Code Review)