On Thu, 16 Dec 2021, Abramo Bagnara wrote:

Il 13/12/21 04:37, Nicolas Pitre ha scritto:

On Sun, 12 Dec 2021, Abramo Bagnara wrote:

Who in its right mind will believe that sprinkling typecasts around
would make the code any safer and understandable? And that's not
mentioning those _double_ typecasts being added too!!! We should instead
aim for cast _reduction_ to let the compiler warn more about type
mismatches and air the code, not add new ones!

Note: Sometimes double casts are necessary, but we do hide them behind
special accessors.

In the specific, double casts are needed in two situations:

1) A char argument to ctype predicates should be first converted to
unsigned char (as the C standard requires) and then converted to
int to avoid mixing of signed and unsigned. Said that, nothing
prevents us from using helpers that accept char argument or to
embed such double casts in, e.g.,

#define CTYPE_CHAR(c) ((int)(unsigned char)(c))

Unless we find a good place for this macro I'd be forced to leave double cast.
If it is preferred I can add a line of documentation before each occurence.

I think <sys/util.h> would be a good place.

And a real kicker is this change:

- * @return 0 on success, nonzero on failure
+ * @return false on success, true on failure

Who really thinks that the above is idiomatic and an improvement in code
understandability? It is not that MISRA does require it in this
particular case, but it certainly did skew the mindset of the person who
did that change.

I'd like very much you take a look to

https://github.com/Abramo-Bagnara/zephyr/commit/87a42d4185828fb1e651604b8ee878063fb6b08a

Please make it into a fetchable branch. That'd make my reviewing easier.


Nicolas