Attention is currently required from: pespin, fixeria.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31612 )
Change subject: cosmetic: clarify test_codec_support_bts() ......................................................................
Patch Set 1:
(2 comments)
File src/osmo-bsc/codec_pref.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31612/comment/67eb48cf_c15be85c PS1, Line 197: switch (pchan) {
I prefer the new version :)
i always prefer switch() for enum values, because they enforce catching odd values.
https://gerrit.osmocom.org/c/osmo-bsc/+/31612/comment/92a42a84_6521a210 PS1, Line 225: return (bool)bts_codec->efr;
You probably want to (bool)(!!bts_codec->efr); to make sure values are 1 or 0?
the return value is a bool, so casting to bool is the logic fit, and even that is just syntactic sugar. I guess when assigning to some 1-bit storage, then it should have the foo_bit = (my_bool ? 1 : 0). But it doesn't belong here IMHO. If we start doing that here, we'd have to add these things all over the place where ever bool is used, just in case some caller might assign to a single bit storage... it's the wrong place to do it. ... also who guarantees that !!(x) always returns 0 or 1.
btw i just checked and test_codec_support_bts() is only used in if(), no single bit storage involved.