Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31544 )
Change subject: commonlib: Add Bubble sort algorithm ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/#/c/31544/3/src/commonlib/include/commonlib/sort... File src/commonlib/include/commonlib/sort.h:
https://review.coreboot.org/#/c/31544/3/src/commonlib/include/commonlib/sort... PS3, Line 16: SORT_H_ nit: _COMMONLIB_SORT_H_?
https://review.coreboot.org/#/c/31544/3/src/include/stdlib.h File src/include/stdlib.h:
https://review.coreboot.org/#/c/31544/3/src/include/stdlib.h@9 PS3, Line 9: #define swap(a, b) do { \ This needs to be in commonlib if the function using it is in commonlib (probably commonlib/helpers.h... that's auto-included from here).
https://review.coreboot.org/#/c/31544/3/src/include/stdlib.h@13 PS3, Line 13: } while (0) Uhhh... not sure how I feel about this. Using lvalues in macros is commonly frowned upon. Using pointers would make it easier to read the macro "like a function". But then again maybe I'm overreacting and swap is exactly the kind of use case where lvalues in a macros are perfectly reasonable.
(If you do decide to change it, please also add extra helper variables to prevent double evaluation.)