Attention is currently required from: Andrey Petrov, Arthur Heymans, Christian Walter, Hung-Te Lin, Jakub Czapiga, Johnny Lin, Maximilian Brune, Ronak Kanabar, Tim Chu, Yidi Lin, Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77969?usp=email )
Change subject: treewide: Add commonlib/bsd/stdlib.h ......................................................................
Patch Set 7:
(8 comments)
File src/commonlib/bsd/include/commonlib/bsd/stdlib.h:
https://review.coreboot.org/c/coreboot/+/77969/comment/17ad0ab9_40b14d69 : PS7, Line 10: #if CONFIG(COREBOOT_BUILD) I think this is also not great (I know that that's not your patch, but I think a bunch of things were badly designed when these files were first added and then hardly anything ever used them so nobody cared much, but now if we want to start using them more we should clean stuff up a bit). CONFIG() isn't guaranteed to be defined in the commonlib environment, and I believe would in fact be missing if you tried to use this from cbfstool.
I think we should replace this with something that comes directly from the coreboot Makefiles (e.g. set `-D__COREBOOT__` in `CPPFLAGS_common` and then check with `#ifdef` for that here).
https://review.coreboot.org/c/coreboot/+/77969/comment/7005b3d8_291079f8 : PS7, Line 30: %s/%s/line %d nit: This may just be my personal nitpick, but I find the `%s:%d %s():` from libpayload nicer than this.
File src/commonlib/include/commonlib/stdlib.h:
https://review.coreboot.org/c/coreboot/+/77969/comment/81fd5b68_e00e32c6 : PS2, Line 44: int dma_coherent(const void *ptr);
Now I removed the `dma_malloc` and `dma_coherent` declarations from commonlib stdlib. […]
Acknowledged
File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/77969/comment/4abee7fd_2b4e65de : PS6, Line 8: #include <commonlib/bsd/stdlib.h>
They originally all included commonlib/stdlib.h so I wasn't sure if that was the intention. […]
Yeah, that policy isn't cleanly implemented everywhere in coreboot, but it's good to fix up things while you touch them. (This file is still wrong though, in the others you got it right now but this should also be `<stdlib.h>` not `<commonlib/bsd/stdlib.h>`.)
File src/lib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/77969/comment/52bb0d8a_d91c507d : PS7, Line 5: #include <stdlib.h> already included below
File src/soc/intel/xeon_sp/spr/numa.c:
https://review.coreboot.org/c/coreboot/+/77969/comment/271d1483_ed355a2c : PS7, Line 4: #include <stdlib.h> Not really necessary for files that already include `<types.h>` (that's our convenience shortcut header for "stdlib, stddef and all that stuff").
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/77969/comment/252752bb_18a1f495 : PS7, Line 4: #include <stdlib.h> same
File src/soc/mediatek/mt8195/pcie.c:
https://review.coreboot.org/c/coreboot/+/77969/comment/977e50be_d14fa718 : PS7, Line 3: #include <stdlib.h> same