You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31606 )
Change subject: libpayload: cbfs: Require input size and output size for cbfs_decompress ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31606/1/payloads/libpayload/include/cbfs_cor... File payloads/libpayload/include/cbfs_core.h:
https://review.coreboot.org/#/c/31606/1/payloads/libpayload/include/cbfs_cor... PS1, Line 257: int srcn
edit: return value too, please. all the functions called from here return unsigned.
Done
https://review.coreboot.org/#/c/31606/1/payloads/libpayload/libcbfs/cbfs_cor... File payloads/libpayload/libcbfs/cbfs_core.c:
https://review.coreboot.org/#/c/31606/1/payloads/libpayload/libcbfs/cbfs_cor... PS1, Line 331: len = MIN(srcn, dstn);
It would just separate error behavior from expected behavior better. […]
I think using assert here is not consistent (it halts while other algorithms just return 0 on error).
'Do memcpy and then return 0 when srcn > dstn' also sounds weird to me because caller only knows 'there is an error' and doesn't know how much bytes can they use, hence there is no need to prepare the partial output. (And it seems like ulz4fn() returns 0 for partial decompression while ulzman() returns the partial decompressed length.)
When srcn > dstn, I think we should either: 1. memcpy(dst, src, MIN(srcn, dstn)); return MIN(srcn, dstn); 2. return 0;
As partial result is not really being used, I think we can just return 0 for now?