I figure that that the MISRA-C changes are an attempt to step in the direction to having a version of Zephyr that is or can be certified.

I've been involved in the certification process (DO-178C, though there are others) for other embedded OSes in the past and they have admittedly been beasts of process. One of the early steps that we had to go through was ensuring coding standard compliance. There is no doubt that it was both time consuming and tedious and sometimes led to code looking a little weird for compliance purposes. Despite that, the benefits to doing so (in the context of certification) were substantial as it helped to open up new business opportunities. Similarly, I suspect one of the longer term goals here is to help open up new Zephyr adoption opportunities.

I realize that the above does not directly address the points of contention that you raised. By the time I am chiming in on this, I believe that others have already responded to them. My primary point that I am trying to draw attention to is how this could fit into a larger beneficial picture, despite the pain (in what is hoped to be "short-term").

Peter

-----Original Message-----
From: devel@... <devel@...> On Behalf Of Nicolas Pitre
Sent: Saturday, December 11, 2021 5:01 PM
To: devel@...; Kevin Hilman <khilman@...>
Subject: [Zephyr-devel] MISRA


<rant mode on>

Could someone tell me what is all this MISRA compliance eagerness for?
I can't believe that no one rebelled against that yet.

I just looked at a proposed PR and this makes me want to cry.

Some of the changes are good, like stricter boolean usage.

But the majority of the changes are pure code obfuscation!

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.

Why do we have to make all loop index vars unsigned? It is a very common idiom to test for "i < 0" in loops. Making some signed and some unsigned is just adding to the inconsistency for no gain.

Why do we need to test against an unsigned zero everywhere? Does anyone know a about a compiler implementation where a signed 0 vs an unsigned 0U makes any difference?

And what is that obsession about qualifying every literal values anyway!
When signed and unsigned expressions are combined, the result is promoted to unsigned already. So I do ask you frankly which of the following is the safest and most intuitive:

#define FOO 5
#define BAR 5U

[ ... code far away ... ]

bool foo(int x) { return (x < FOO); }
bool bar(int x) { return (x < BAR); }

Notice how foo(-1) and bar(-1) don't give the same answer. Try it if you don't believe me.

Whereas:

bool foo(unsigned int x) { return (x < FOO); } bool bar(unsigned int x) { return (x < BAR); }

Here foo(UINT_MAX) and bar(UINT_MAX) both do give the proper result.

So qualifying literals everywhere when it is not absolutely required is actually evil and opens the possibility for *more* bugs not less.

And why casting pointer difference to a size_t? The official type for a pointer difference is ptrdiff_t and the standard offers no explicit guarantee that sizeof(ptrdiff_t) equals sizeof(size_t). People who live by the standard will tell you that size_t _could_ be smaller and therefore silently truncate the result.

What's the point of casting to a char before assigning to a char when the cast is implicit already?

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.

Etc. Etc.

<rant mode off>

So I really do challenge if MISRA compliance is a good thing for the project. Some of it certainly makes sense. Some of it is really questionable and should be excluded.

I do guess (even if I don't understand why in the end) that some Zephyr users do require MISRA compliance for their project. I however do believe that this is not beneficial for the Zephyr community as a whole, and that the cost of MISRA compliance and its side effects should not be imposed on everyone without further scrutiny and discrimination.

Static analysis tools have come a long way and they are far more useful, realistic and relevant when it comes to code validity and correctness at this point.

Many embedded libraries and SDK's out there already maintain a MISRA compliance exception list. I think this would be a sane thing to do and that wouldn't be a first.


Nicolas