Attention is currently required from: Elyes Haouas, Martin L Roth.
Angel Pons has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/82713?usp=email )
Change subject: util/lint: Warn if {stddef,stdint,string}.h included but not used ......................................................................
Patch Set 3: Code-Review-1
(2 comments)
Patchset:
PS3: I fail to see why this change would be worth it.
This assumes that indirect header inclusion is never a thing. However, there is at least one header that indirectly includes some of the headers here. Maintaining a list of excluded headers would only add to the maintenance burden.
IMHO, the maintenance burden alone outweighs any benefits of automatically detecting unnecessary includes of a few headers. coreboot compilation times are relatively short (especially with ccache).
The few cases where I've seen coreboot rebuild times go up noticeably (still being well below 30 seconds on my machine) is when changing stuff that impacts the devicetree on newer Intel platforms (nearly all of the SoC code needs to be recompiled) or headers that get included from many places (but that's kind of expected).
File util/lint/lint-stable-032-includes:
https://review.coreboot.org/c/coreboot/+/82713/comment/5147e2b8_8035a6d5?usp... : PS3, Line 19: HEADERS["<stddef.h>"]="size_t\b|ptrdiff_t\b|NULL\b|offsetof\b|wchar_t\b" : HEADERS["<stdint.h>"]="int8_t\b|int16_t\b|int32_t\b|int64_t\b|uint8_t\b|uint16_t\b|uint32_t\b|uint64_t\b|intptr_t\b|uintptr_t\b|u8\b|u16\b|u32\b|u64\b|INT8_MIN\b|INT16_MIN\b|INT32_MIN\b|INT64_MIN\b|UINT8_MAX\b|UINT16_MAX\b|UINT32_MAX\b|UINT64_MAX" : HEADERS["<string.h>"]="memcpy\b|memmove\b|memset\b|memcmp\b|memchr\b|strdup\b|strconcat\b|strnlen\b|strlen\b|strchr\b|strncpy\b|strcpy\b|strcmp\b|strncmp\b|strspn\b|strcspn\b|strstr\b|strtok_r\b|strtok\b|atol\b|strrchr\b|skip_atoi" IMHO, having to maintain these lists manually is wasteful. It's true that they don't change often, but any missing elements from this list would produce a false positive.
And if there's multiple versions of these headers with the same name (e.g. coreboot's `string.h`, vs. the host's `string.h` used in `util/`), this can result in irreconcilable misdetections (either false positives or false negatives).