Change in coreboot[master]: cbfs: Add helper functions to look up size and type of a file

[Julius Werner (Code Review)]

Attention is currently required from: Jakub Czapiga, Werner Zeh.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59312 )

Change subject: cbfs: Add helper functions to look up size and type of a file
......................................................................

Patch Set 1:

(3 comments)

File src/include/cbfs.h:

https://review.coreboot.org/c/coreboot/+/59312/comment/1669e976_85b86f6b 
PS1, Line 134: static inline bool cbfs_file_exists(const char *name);
             : static inline bool cbfs_ro_file_exists(const char *name);
> I wonder, if we actually need these functions. […]

Here was the original code that highlighted the need for a helper like this: https://review.coreboot.org/c/coreboot/+/55367/52/src/soc/intel/elkhartlake/...

Sometimes you may need to check if a file is there without immediately loading it if it is (and like I tried to point out in the comments, we should avoid those situations as much as possible, but sometimes they can't be avoided).

https://review.coreboot.org/c/coreboot/+/59312/comment/65465664_dbc082b4 
PS1, Line 308: static inline size_t cbfs_get_size(const char *name)
> Should not these functions be inaccessible for code running in CAR mode and similar? This might make […]

It's the same stack requirement as a normal CBFS file load? Early stages do those as well. The general coreboot minimum stack size is 2K (and I believe in practice all platforms are much larger... for x86 platforms the CAR stack tends to be 8K or more). cbfs_mdata is intentionally limited to 256 bytes since that should hopefully never be an issue.

File src/lib/cbfs.c:

https://review.coreboot.org/c/coreboot/+/59312/comment/4c9c3d86_07a45654 
PS1, Line 528: if ((err = _cbfs_boot_lookup(prog_name(pstage), false, &mdata, &rdev)))
> Would not it be cleaner to set err value in a separate line above? Personally I find assign-and-chec […]

I'd say it's a personal style choice... it's not forbidden by the coreboot style guide. I find them useful and increasing readability, because the same code can be written in fewer lines (so you can have more meaningful parts of a function on one screen). Also, I think they are much more important for while-loops, and when you allow them in while-loops there's no reason not to use them in if-statements where appropriate as well. (For example, writing a loop like https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src... where you want to call a function, use the result to decide whether the loop ends, and then continue working on the result if it doesn't, looks a lot more ugly and harder to follow without assign-and-check.)

Also, in my experience, most concerns about this just come from some 90's era conventional wisdom that it was dangerous (accidentally writing = instead of ==), but these days GCC forces you to put extra parenthesis around the assignment anyway so doing it unintentionally is impossible.