[TangerineSDR] Fwd: Proposal: HamSCI Code Review

Rob Wiesler robert.wiesler at case.edu
Thu Aug 13 15:36:06 EDT 2020


> On Wed, Aug 12, 2020 at 10:35 PM Jay Schwartz <wb8sbi at gmail.com> wrote:
>> In the auto industry code reviews are mandatory, and misery unto those who do not do one.  Part of this code review is to make sure the code complies with the Motor Industry Software Reliability Association (MISRA) standard.  This is a coding standard that started in the auto industry and has gone worldwide in many other industries.  The standard can be purchased for 10 British Pounds from https://www.misra.org.uk/
>>
>> Many software compilers have incorporated this standard into their tools.  They will automatically check the source code at compile time and report any non-complicenses.  I.e. your code compile and link, but may not run the way you think it will.  By complying with this document you greatly reduce the chances of something undesirable happening.
>>
>> This document is easy to comply with, and contains a lot of common sense do / do nots.  By complying with it also makes your code more readable and better able to be updated.  However, it is not friendly to spaghetti code.
>>
>> If the code is written to comply with it then many preventable bugs can be avoided.  I.e. it reduces the need to patch by not having the bug in the code to begin with.
>>
>> I have a copy of the standard and would be happy to explain it to the group at the next regularly scheduled video meeting.

"The nice thing about standards is that there are so many to choose from."

Jay, I'm afraid that MISRA is not a good fit for this project.  This
is a volunteer, open-source project (although I take it that not all
code reviews will necessarily review such code).  That means that the
resources, tools, and documentation used by the project must be freely
available to all (potential) volunteers.  "Free" in this case means
both "free as in beer" and "free as in freedom" - what use is a
standard if it can't (legally) be copied, spindled, mutilated, and
distributed to anyone who might be even merely curious about the
project?  It doesn't actually help if you provide some sort of
(limited/verbal) access to your copy - we still have a legal problem,
and new/potential volunteers won't have that access.

On top of that, MISRA has a few more large problems:
- It's for C/C++ only, whereas we're likely to have significant
Python/shell components.
- It's a product of the automotive industry, which came to software,
and still lags behind best practices and modern design.
- It's not actually a particularly great standard.  While I have only
partial access to the standard, those who have bothered to shell out
the 10₤ just to look at it don't have good things to say ([0]).

[0] https://en.wikipedia.org/wiki/MISRA_C#Criticism

While I understand that having a standard to compare code against
seems attractive, I have to warn against it.  MISRA (and other coding
standards) try to be good, formalized knee-jerk reactions against
things likely to be wrong or cause subtle bugs.  There is no
substitute for experience.  No document can come anywhere near the
knee-jerk reactions of an experienced programmer, which say things
like "it's suspicious that you're calling a blocking function from
inside the main event loop" instead of useless pedantic trivia like
"oh no, the right operand of your && expression has side-effects, that
sounds iffy".  Software craftsmanship is not something you can boil
down to a set of simple rules, except for the simple things that are
already checked for by GCC's "-Wall -Wextra -Werror" flags.  The
experienced programmer will not benefit from such a document, and the
inexperienced programmer risks picking up cargo-cult behavior.
Furthermore, it's better to just have a good software review, which
not only catches problems, but also proposes solutions.

If anyone feels compelled to use a document to help them critique
their own code, I recommend Strunk and White's "The Elements of
Style".

A separate question is whether or not we want to adopt any particular
style guide.  I hate that question, because the most important aspect
of any style guide is that it be opinionated, and as a result
basically all published style guides are terrible in some way.  As
long as we get rules 1 (maintain consistent style within a single
file, and ideally within a single project) and 2 (don't use tab
characters after non-tab characters within a line) down, I probably
won't complain too much, and in any case, if people are bringing their
own code (possibly unrelated to TangerineSDR) to the code review, the
question isn't really pertinent, anyway.



More information about the TangerineSDR mailing list