Re: MISRA-C and Zephyr


Himanshu Jha <himanshujha199640@...>
 

Hello Flavio,

On Fri, Sep 14, 2018 at 11:19:40PM -0700, Flavio ceolin wrote:
Agree, we have to automate it as much as possible. But we can't just wait
have all tools ready to start fixing the issues. Currently there are almost
8k violations on kernel. Any tool we setup will make a lot of noise.

Some rules we will be able to check with coccinelle, but ideally we need a
qualified tool with Misra c support.
Thanks for using Coccinelle for the memset cleanup!

Coccinelle can handle MISRA-C compliance at some extent and it really
depends on the what you wish to accomplish through a coccinelle script.

Some scripts are easy to write, but certainly we can't claim to write
scripts with 0% false positives. It depends upon the problem set and
how well you design a cocci script.

For instance: We planned to remove the unnecessary casts returned by
memory allocating functions *alloc since we know the conversion
from void* and other type is implicit.

https://github.com/torvalds/linux/blob/master/scripts/coccinelle/api/alloc/alloc_cast.cocci

But while working and testing the script, I found some strange code
where the cast was necessary and removing it would be a regression:

drivers/scsi/fnic/fnic_trace.c

<snip>
static unsigned long fnic_trace_buf_p;
...
fnic_trace_buf_p = (unsigned long)vmalloc((trace_max_pages * PAGE_SIZE));
</snip>

And some other case as well, where the cast was required...

Therefore, we removed those cases by embedding a python script into the
coccinelle script(yes, we can embedd python & Ocaml script if required)

What I'm trying to explain is that some cases are easy to handle while some
are disaster to get desired property.

Also, we classify accuracy of the Coccinelle by defining a property
called "Confidence" at the top of the cocci scripts.

Confindence: High --> very rare chances of false postives.
Moderarte --> some chances of false postivies.

We always try to improve the cocci script as much as possible.

If there is any MISRA-C compliant build system then its obviously great.
But if not, then my proposal would be add `coccicheck` to CI and write
cocci scripts to warn.

For now we have some ideas that we are going to inherit from the
mainline kernel like:

* ARRAY_SIZE: Lots of instances in Zephyr where it is hard-coded rather
than simply using the ARRAY_SIZE helper macro.

* GENMASK: again we could use the helper macro instead of
BIT(3) | BIT(2) | BIT(1) == GENMASK(3,1)

And this GENMASK defintion is defined in:

drivers/dma/dma_stm32f4x.c:#define GENMASK(h, l) (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

So, we could probably, move the declaration to include/misc/util.h
and use it throroughly on the codebase.

...and few more.

Many of the rules we made for mainline is based on enforcing usage of
available standard APIs and thus avoiding developers to roll-out their
own wrappers. In this way we can enforce new incoming code to adhere to
the Zephyr APIs and prevent any subsequent violations/redundancy.


https://github.com/zephyrproject-rtos/zephyr/pull/9725


Thanks
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Join devel@lists.zephyrproject.org to automatically receive all group messages.