Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31346 )
Change subject: libpayload/sys/types.h: Add definition for off_t ......................................................................
Patch Set 2:
Generally, implementing standard things in non-standard headers can only lead to one thing, IMHO: At some point, you'll realize that you have to use the standard headers (like in this case, to be able to compile code that doesn't target libpayload explicitly) and when you try to fix the mess too late, you'll cause breakage in the code that does target the non-standard environment explicitly.
Okay, I don't really care that much. But we should at least make sure that the "include libpayload.h to get everything" use case also still works, so you should add an include for this to that (maybe in place of <arch/types.h>).
Done. But as an independent include, as the indirect include may not live long (follow-up).
Also, why not define it to ssize_t?
Good question, have to look that ones definition up... ok, nothing special. I see no reason against it, also no reason to use it. May I ask why?
Just because I like relying on the exact definitions of what a "long" is as little as possible, maybe that's just a personal preference. With the whole LP64/ILP64/LLP64 thing I'm always getting terribly confused about what the standard integer types mean on a certain architecture, so I think it's nice to rely on well-defined types that are set up in a single place in arch/types.h as much as possible. (I know libpayload already isn't too great with that to begin with, including the definitions of size_t and ssize_t... I'd just prefer to not add more.)
But wouldn't a relation between `ssize_t` and `off_t` make it even more confusing? Generally, you don't know their size. So planting the idea that they are the same here, might con- fuse us again when we work with other definitions.
I think it's best to never assume to know the width of these types.