Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforcement of the coding conventions #190

Closed
pavel-kirienko opened this issue Dec 19, 2018 · 5 comments
Closed

Enforcement of the coding conventions #190

pavel-kirienko opened this issue Dec 19, 2018 · 5 comments

Comments

@pavel-kirienko
Copy link
Member

The coding conventions are described here: https://kb.zubax.com/x/84Ah

The existing codebase has certain deviations which should be fixed in the rewrite; the two things that come to mind are the capitalization rules (CANFrame is correct, CanFrame is not) and reliance on the native types (std::uint32_t is safe to use, unsigned is not). Also, a lot of useless stuff can be safely thrown out now since we have agreed to abandon support for C++03.

We need some tool for enforcement of MISRA C++ rules. I Googled around a bit and found nothing except SciTools Understand, which I have a bit of experience with: https://scitools.com/features. Back in 2014 they offered a free license for open source projects, not sure if they still do. While the tool generally worked for me, it was rather unreliable and tended to produce nonsensical reports, especially when dealing with heavily templated code which libuavcan has much of. Seeing as we have no other options so far, we should probably give it a try.

Besides MISRA, we could well use more static analysis in general. We currently have Coverity Scan; perhaps we should also consider using the Clang Static Analyzer, which I heard good things of but never used myself.

@pavel-kirienko
Copy link
Member Author

@thirtytwobits SciTools bestowed one free license upon us. It is valid for one year, can be freely renewed afterward.

I asked them whether it is possible to set up their tool in a headless mode in a CI environment, but they ignored this question twice, so I take it as a "no".

@thirtytwobits
Copy link
Contributor

This task has become a bit too non-specific for v1. We now have enforcement of style and Issue #226 covers integrating clang-tidy but we do lack MISRA and/or HIC++ enforcement. Can I change this to read "Choose and enforce a C++ standard for safety critical software"?

@pavel-kirienko
Copy link
Member Author

I think I might have found just what we need. SonarQube has a cloud-based static analysis service named SonarCloud which is free for open source projects, and it supports a reasonable subset of MISRA (https://rules.sonarsource.com/cpp/tag/misra, https://rules.sonarsource.com/cpp/tag/misra-c++2008) among other rules. I have set it up for the new PyUAVCAN rewrite which is my first ever encounter with this service; currently I can't vouch for its C++ side so we need to try it. The list of rules at least looks very promising.

There is also Coverity Scan, which AFAIK does not offer MISRA checks in its free version but its in-depth control flow analysis has proven to be generally useful in the past (we're using it for one project internally). I am not sure, however, how does it compare against clang-tidy; experimentation might show that once we've set up the latter Coverity might become redundant.

I believe you have already taken care of the coding style enforcement with clang-format so that is a non-issue anymore.

Per the MISRA usage guide we need to set up a dedicated requirement document containing a list of rules from all applicable coding standards (such as MISRA or HIC++) with an explicit list of tools per rule that enforce it (which would be (so far) SonarQube, Coverity, clang-tidy, clang-format). Would you consider adding that to the libuavcan docs?

@pavel-kirienko
Copy link
Member Author

pavel-kirienko commented Feb 15, 2020

Integration examples for clang-tidy, clang-format, and SonarQube with MISRA enforcement are available in https://github.com/pavel-kirienko/o1heap. The coverage measurement is quite flaky there but it's a different beast and I think it's solved here properly already.

@pavel-kirienko
Copy link
Member Author

Seeing as SonarQube is set up, I suggest we split this issue into two new ones:

1. Enable MISRA checks in SonarQube

A large subset is supported and can be enforced using SonarQube:

image

2. Set up Clang-Tidy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants