Hi Rasmus, thanks for the review! > I know it's prototypical bikeshedding, but what's with the "get_" > prefix? Certainly the purpose of any pure function is to "get" the > result of some computation. We don't have "get_strlen()". Hmmm, can be argued. The reason I chose the naming was that the open coded implementations were sometimes either returning the current parity or returning the bit which you need to enforce a certain parity. And this was never obvious from their names. Sometimes it was not even clear if the returned bit was for odd or even parity, just for "their" parity whatever it was. So, with "get_" I wanted to ensure that people can read from the name, that this is about the current parity and one has to draw conclusions from there. But maybe the kdoc documentation is good enough for this? > Please either just "parity8", or if you want to emphasize that this > belongs in the bit-munging family, "bit_parity8". I'd go for 'parity8' then. > > + * if (get_parity8(val) == 0) > > + * val |= BIT(7); > > Shouldn't that be ^= in general? Of course in some particular > application one may have constructed the value in the lower 7 bits and > "know" that bit 7 is currently clear. Yeah, that is my use case. True, your example is more generic. Will fix. All the best, Wolfram
Attachment:
signature.asc
Description: PGP signature