Attention is currently required from: Arthur Heymans, Julius Werner, Paul Menzel, Yu-Ping Wu.
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78798?usp=email )
Change subject: commonlib: Add GCD function ......................................................................
Patch Set 3:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/78798/comment/588a39a0_aa98441a : PS2, Line 7: lib: Add GCD function
path is no longer "lib"
Done
Patchset:
PS2:
Can we add some tests?
Done
File src/commonlib/bsd/gcd.c:
https://review.coreboot.org/c/coreboot/+/78798/comment/64b31191_c4218a1d : PS2, Line 6: unsigned long gcd(unsigned long a, unsigned long b)
Looks like all the use cases we have are 32-bit and on x86 doing this at 64-bit would be expensive, […]
gcd32 sounds good to me
https://review.coreboot.org/c/coreboot/+/78798/comment/ade8c89d_9ba0d1a7 : PS2, Line 10: if (a == 0 || b == 0)
Shouldn't we just let these cases run into the divide-by-zero? I wouldn't consider asking for the GC […]
Done.
https://review.coreboot.org/c/coreboot/+/78798/comment/4984ad14_e1c410c0 : PS2, Line 12: if (a < b)
I think writing this explicitly is unnecessary. […]
Removed
File src/commonlib/bsd/include/commonlib/bsd/gcd.h:
https://review.coreboot.org/c/coreboot/+/78798/comment/aaa9d894_c14a56fa : PS2, Line 3: #ifndef __GCD_H__
nit: Include guards should include the COMMONLIB_BSD somewhere so that there won't be any clashes in […]
Done
-
Yidi Lin (Code Review)