Coding standards help keep the code clean, and consistent. The guidelines below also explain and cite the reasons why we should follow them, and certain ones will have references to ISO26262, which is a safety certification guideline for code.

Goals

• Consistency

• Promote Best Practices

• Provide Structure to the code

• Avoid bugs and increase developer efficiency

• Optimize for code reviewers and readers rather than code developers

Non-Goals

• Nitpick on the developer code

• Argue whitespace issues (clang-format auto formats brace positions and spaces)

Any code you write, modify or generate should follow the coding standards. If you find any code that doesn't follow these rules, please take the initiative to fix it.

Follow the Boy Scout Rule

Leave the campground cleaner than you found it.

Principles are guiding rules to be more successful. You need to constantly keep these in mind and have them serve as a guardrail to keep you focused towards writing good, and clean software.

You should not learn of the following principles once and then forget them. A principle is a kind of rule, belief, or idea that guides you.

[Clean C++]

Keep It Simple and Stupid and less obscure. Keep the logic as simple as possible, and oftentimes as un-optimized as possible. Aim for code readability over creating fancy logic.

Very cool macro but could be deadly:

Violation of the KISS principle in an effort to "optimize":

Aim for extreme simplicity and write minimal amount of code. And remember my wise words:

It is not about how many lines of code you write, it is about how few.

-- Lockheed Martin (Preet)

Copy and paste is a design error.

-- David L. Parnas

Do not Repeat Yourself The extent of this even means not repeating yourself in comments. In general, whenever you believe a change in logic requires you to make the same fix somewhere else, then it is time to refactor the logic to avoid repeating yourself.

Do not repeat yourself in comments either

But apart from this silly example, do not repeat your code in multiple places. Instead, refactor to a common function, and then re-use that. Effects of this includes:

• Avoid updating a bug-fix in multiple places

• Unit-test just one piece of code

• Maintain just one source of truth

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

-- The Pragmatic Programmer

You Ain't Gonna Need It This one is the most occurring theme among developers. We usually develop APIs that we think we will need one day. But the fact of the matter is that we can design the APIs when we really need it.

Here is a good one:

Do not do something today that you can do tomorrow, because tomorrow, you might not need to do it at all.

IBM Websphere Group (Preet)

Prefer to build software when you really need it, otherwise it is a waste. Avoid your impulses:

• I might need it one day

• This will make it better for the future

But at the same time, provisioning certain things towards the future is okay. For example, the TCP/IP stack provisioned for some reserved bytes and although the standard is largely unchanged, it did provide the roadway towards IPv6. Another example is that if you are developing a communication standard, it is okay to provision for an extra byte for a version that allows you to amend the standard in the future.

Here are some general Sibros guidelines for being a good, responsible engineer.

• Write code for someone else to take over

  • Strong developers never need to worry about job security

  • Pass it on such that you can go do even more exciting work

  • Knowledge is not worth anything until transferred

• Take your time, and do things right

  • Write code as if you are writing safety critical code for a pacemaker or an airplane flight controller

  • This includes unit-test code; write maintainable unit test code

• Apply proper code fixes rather than patches

  • Any workarounds are going to come back to haunt you

• Be thorough, and understand your code fixes

  • Get to the root cause

  • There is no such thing as magic

• Have a firm definition of it works

  • Can you bet your life on it?

  • Will it work 100% of the time?

• Write high quality, deterministic code

  • Design code such that you will never need to hit the reset button for 1 year, 10 years, and beyond

  • One Sibros team member's previous colleagues once said Oh we hit this bug every 24 hours, let's reset our controller every 23 hours

    • They never worked with that person again, and another team member gets angry if they ever mention his name

  • Memory allocation should never fail

  • Design code such that even if someone attacks your controller through a CAN bus, or Ethernet network, your code will always result in predictable execution

• Work with integrity and honesty

  • Treat your co-workers and stakeholders with respect, even when in disagreement

  • Communicate concisely and ensure criticism and comments are constructive

  • Don't be afraid to give honest feedback, and continue to iterate and improve on your work

• Start with establishing a good base for your header file before writing your code

  • All parties can work together to come up with a solution before implementing any code; more ideas are always better!

  • Reviewing the header file first clarifies any issues with ticket requirements or misunderstandings

  • This also prevents wasted time developing code, tests and debugging said code that may end up being deleted

Modularize code, and do not create God Objects. When you are dealing with a large problem, split the problem into smaller code modules and make the code only do one thing. This allows for the module to be implemented with simple logic thus improving the overall code readability as well as making it easier to test.

If we are creating an HTTP client, do not put all the code into a single module. Instead, consider splitting this problem into small code blocks, such as:

• A socket library to read the data

• A buffer module that the data is read onto

• An HTTP parser library to parse the incoming data

Reference this article for further inspiration.

• Write Testable Code

  • Write code with the test code alongside. This is hard to capture in a guideline like this coding standards' document, but design code that is modular, and testable. Refer to the C Unit Test Tutorial for more details.

• Keep the functions small

  • Do not write functions that are larger than 10 lines of meaningful code. The benefits are:

    • Creates smaller units of code that are easier to test

    • Increases code clarity

    • Decreases code complexity (ISO26262, Table 1: 1A)

• Do not use more than 5 brace levels in your code

  • Refactor code into smaller functions

  • Re-structure the branch statements to reduce the curly brace nesting

  • This improves the overall readability of the code

• No recursive functions

  • In Embedded Systems, the stack memory is precious, as there is usually not very much.

• Each function should only have one return statement

  • and that should be at the end of the function. This is to prevent unintentional side-effects where a developer adds some code thinking it will be run, but the function exits out before it gets to that piece of code.

  • MISRA and safety certified code requires this

• Use (void) for functions without inputs

  • A function with no input should be labeled as func(void) for the parameters. This is because if you omit the void in C, you can pass it infinite number of parameters. C++ safeguards you for this case though.

• Prefer at most 10 meaningful lines per function

  • This forces you to make the code more modular

  • This helps during unit-testing

  • Avoid using more than 5 {} brace levels in your code

• Use Yoda notation in equality checks but not anywhere else.

  • Example: Use if (5 == value) instead of if (value == 5)

  • Example: Use if (const_x != value) instead of if (value != const_x)

  • Example: Do not use if (5 < value); see Yoda notation

  • Example: Do not use if (10 >= value); see Yoda notation

  • Because we don't want to accidentally assign the variable in the condition check.

  • Even though static analysis should catch this, we do not want to rely on it.

  • PCLint currently detects and fails on accidental assignment in condition for both C and C++, so Yoda notation is no longer required (but still recommended)

• Use const type qualifier on pointer parameters when applicable (i.e. when the referenced value remains unchanged).

  • Do not use this type qualifier on pass-by-value parameters as it is redundant.

Assuming there is a heavy use of pointers, we want to balance the fundamental checks with those that may actually be useful and create a safer code-base.

C code modules are designed based on structs and the instances of these structs are then given to APIs. This mimics how C++ classes operate as the first parameter to functions is the module pointer itself which C++ refers to as the this pointer.

• Validate and check all pointers being passed into public API

  • We used to avoid this, but had to revert our decision based on safety certified code requirements. It introduced extra brace level and increased cyclomatic complexity, but it was necessary, and hence the next bullet point is striked-out.

  • Do not check the this pointer equivalent of C++ with NULL. This creates a lot of fluff, creates potential waste that will only be rarely useful because it is a programming error to pass NULL pointers and we cannot be over-protective about this condition.

• Private functions of the module can omit additional pointer checks as long as you can ensure through unit-testing that those pointers could never be bad memory pointers

• The rest of the pointers should be checked and design code such that error conditions will not end up in a CPU exception.

There may be reasons for not checking the this_ptr, however, the safety aspects of the code outweigh the benefits. Here are some downsides that we did assess though:

• Unit-tests double since each method needs a test for the NULL pointer

• Each function would have additional brace level (early returns are forbidden due to MISRA/C)

• We (used to) consider passing a NULL pointer as a user error, and do not believe it adds significant value

Each pointer must be validated, and there are certain rules to follow in order to get past static analysis tools flagging your code, and preventing it from getting merged.

If there are "private" (or static) functions, they do not need redundant checks as long as code structure guarantee that they will never be called unsafely. These are "forgiven" by PCLint or CERTC as long as they follow the coding standards of containing __private_ name. Here is an example:

Do not re-use variables, and avoid changing the variables passed as copy. You can opt to make the parameters const to enforce this.

C standards call for initialization of all global memory to zero, and therefore, there is no need for explicit initialization. For any customer integration that fails to do this, we need to work with them to make sure that their startup code performs this necessary step.

Initialize the members by name. Use colon for C++ (it doesn’t support C dot initializer)

Example

For consistency reasons, when the dot initializer is not possible, then always use sl_utils__memset_zero(), then set the selected (or all) members appropriately.

The reasons to use sl_utils__memset_zero() is that:

• The notation *s = (struct_s) { } is not portable because some compilers may complain about empty { }

• memset() may be used but it creates possibility of bugs because the parameters of this function may be swapped easily

  • Note that bzero() is not recommended since it has been deprecated

Do not confuse the struct initialization with array initialization which can use = {0}

• char array[2] = {0}

For constant arrays that map two sets of values and should contain a certain number of elements, use a static, compile-time assertion macro to validate the array size.

This ensures that the code will only compile when the number of elements in the map/array equals the number that is expected.

Example For an array that maps enums to strings:

Consider adding the following compile-time assertion:

• No unreacheable code

  • You should not be able to do this at Sibros code base because of enforcement of 100% test coverage

• No dead code

  • Do not leave commented code base that is not used in production

• No recursive functions
  In Embedded Systems, the stack memory is precious, as there is usually not very much.

• No dynamic memory allocation (MISRA C:2004, 20.4) i.e. you are not allowed to use malloc and free as it can cause unintended issues when functions such as malloc are unable to provide a valid memory location.

• No continue or goto statements (MISRA C:2012, 15.1)
  Because even the compiler itself cannot prevent stack corruption for this code:

• Switch Statements

  • There should always be a default in the switch case. The default shall be the last statement in the switch block

  • Switch statements containing fall-through blocks require explicit comment acknowledging the intentional fall-through. To automate this, there is a FALL_THROUGH_SWITCH_CASE macro in sl_common.h that should be used to signal this intent.

• Initialize all local variables - empty pointers should be initialized to NULL

  • Initialize arrays using: int array[3] = { 0 };

  • Do not do this:
    int array[3] = { };

• Prefer defining consts using this approach which is part of <stdint.h>

  • const uint32_t VALUE = UINT32_C (800000);

  • Avoid doing this though: UINT32_C(2 * 2) because this macro simply concatenates UL or similar based on the machine and the result of an operation occurring inside of the macro parameter may not be deterministic

• Use parentheses to make code clear and explicit.

  • Do not do: if a > b + c || d < e

  • Instead do: if ( (a > (b + c)) || (d < e) )

• Conditional Operators Even one line statements should have curly brackets Example:

• if statements should always have an else statement if there is an else if statement. The else statements help explicit indicate that we have thought about that situation and decided that no work needs to be done there. The compiler is smart and will optimize this code out anyway, so no need to worry that this takes code space. Pattern:

• No unary operator if pre or post increment can alter the behavior of the code. Using these operators where there is only the increment or decrement operation is taking place is okay.

• There should be no commented out code in the code base.

  • No #if 0 blocks

  • No block comments that disable code (source control is your friend)

  • This improves the readability of you code and reduces the amount of unnecessary code in the code base.

• Unsigned numbers should have U at the end. Example: 42U

• Unsigned long numbers should have UL at the end. Example: 56UL

• Do not write obfuscated 'smart' code - saving on the microseconds of processor time (or not even that) is not worth the headache of trying to understand someone else's tricky code which can take a long time and also be error prone.

• Use static const in place of const for local variables:

  • Latest decision:

    • static local variables are not allowed according to Module Design

    • This rule caused confusion on the use of static const

    • The benefit of variable data construction outweighs the cost of explanation of this rule

  • Obsolete decision:

    • For local variables, avoid using: const char foo[] = “bar” const int test = 1

    • Use static const char foo[] = “bar”
      Because if we use just const (without static), the variable will need to be constructed on each entry to the function, and would not only use extra runtime, but also use stack memory.

• Do not use the C++ style Expects( ) and Ensures( )

  • We want our code to gracefully fail and not run into exceptions that it cannot handle. Whenever there is doubt, always assume you are designing a safety critical controller that is responsible to ensure that a car’s driver would always be able to apply the brakes correctly on the vehicle.

• Code should be self-documenting

  • Although, there may be code such as mathematical formulas that still require a comment to explain the intent of the code. In this case, make sure to write the comment at the time the code is written

  • Comments can be used to explain the intent of the code if self-documentation is not sufficient

• Include all separator comment blocks (even when there is no content below them) such as:

  • Defines, typedefs etc.

  • Please see coding file templates and example structure below

• Use /** to document multi-line comments for Doxygen

  • For consistency, header files (public APIs) should always do this over /*

  • This is for consistency and also because Lint/MISRA does not like multi-line comments using //

  • Documentation is mandatory for public facing interfaces and optional everywhere else.

  • See Doxygen Tags

• Doxygen doesn’t like documenting static inline functions so do not use them

  • This is also discouraged because inline functions cannot be mocked during unit-testing, as mentioned previously

• For consistency, always use only two types

  • /** style for functions or documentation prior to a construct

  • ///< for documenting something on the same line

• Do not mix of /// and /**

  • The header file looks more consistent this way

• Do not use @brief

  • It simply starts a new paragraph.

  • It has no effect on the documentation. If you want multiple paragraphs, separate them by an empty line

• Do not use @file my_file.h

  • Doxygen knows your file's name so follow the DRY principle

• If there is no parameter or return type, then do not use @param none or @return none respectively

  • @param is only needed for parameter names that are not clear

  • If the variable is very clear and explicitly named, there should be no need for explaining the parameter. If someone does still explain the parameter for whatever reason, we don’t need to spend any time pointing it out and/or having them remove the comment or amend their commit (all of which takes more time than just leaving a redundant comment in there).

• Avoid Comments - comments should only be used if your code itself cannot express the intent.

Truth can only be found in one place: the code.

-- Robert C. Martin, Clean Code

This means that the names of the variables and functions etc should describe their purpose instead of having to rely on comments to understand the code. Spend extra time choosing your variable names, and your goal should be that you do not need to document your APIs. Note that in this code sample, we do not need to describe the parameters.

Here is a bad example

• No Redundant comments If the code can be read and interpreted, do not repeat that in a comment itself.

• No closing brace comments Refactor code such that the braces do not enclose a large block of code and hence there is no need for closing brace comments.

• In general, use comment block style comments (historic c-style) for consistency; /* comment */.

• There should be absolutely no TODO tags in the comments unless accompanied by a Gitlab ticket

• Use /* style comments for multi-line comments (advised by Lint)

• Use /** style comment to comment function docstrings (See Doxygen Comments)

• Do not use /** style comments for documentation inside of functions (/** is for Doxygen)

• Use /**< style comments to document struct members on the same line

API documentation (doxygen style):

Lint exception:

Implementation details that may not be obvious to other developers:

Commenting with ticket for future improvement:

Temporary test code:

An incomplete implementation (resolve before merging, otherwise provide ticket #):

Keep a balance between variable length and making the intent clear.

• Code completion can help

• Avoid exceeding the line character limit

• Do not use a very long name such as:

  • SL_UDS_SERVICE_SECURITY_ACCESS_DIFFERENCE_BETWEEN_REQUEST_SEED_AND_SEND_KEY

• Use this instead:

  • SL_UDS_SERVICE_SECURITY_ACCESS_SEED_TO_KEY_OFFSET

The definition of a magic number is when a hardcoded constant in code is obscure, and the developer's intent is not clear. In other words, readers may ask why was this specific constant chosen? And, what are the effects if I modify the constant?

Developers should avoid magic numbers by ensuring that the intent of any hardcoded constant is clear.