Embedded C Coding Standards

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

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++]

KISS

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)

DRY

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

YAGNI

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.

Best Practices

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

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.

Functions

• 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.

Validate Pointers

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

Pointer Validation Recommendation

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:

Variables

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

Data Initialization

Globals

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.

Structs

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}

Arrays/Maps

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:

Safety and MISRA

• 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.

Documentation

• 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

Examples of good comments:

API documentation (doxygen style):

Lint exception:

Implementation details that may not be obvious to other developers:

Commenting with ticket for future improvement:

Comments needing to be resolved before merging:

Temporary test code:

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

Balanced length for variables

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

Avoid magic numbers

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.

Timeout example

Error code example

Size example

Again, magic numbers are defined as obscure constants, so don’t do the obvious. For example, this statement is obvious and unnecessary:

Avoid vague names

Avoid these:

Use Relative Includes

Shown below are two ways to include a code module. We recommend going with Option 1 for the reasons mentioned below.

• If files move around, we will not have to change code dependencies

• If there are conflicts to file includes (customer common.h vs. our common.h) then we have plans to mitigate the problem

  • Our code modules have layer names, and the chance of conflict are minimal .i.e: sl_common.h

  • One time software integration offsets the cost of relative includes

Module Design

• Some of the main things included in a great module design are: data hiding, full code testability and compile time configuration of the module (so there are less pointers and no dynamic memory allocations, etc).

• One producer, many consumers

  • Only one module should be allowed to set (i.e. produce) a particular variable. For example, only the seats module should be allowed to set the value for the seats_occupied variable.

• No public data

  • If the variable is required for some other behavior outside of the seats module, then it should only be accessed via get functions.

  • Use accessor functions to access data that is internal to a module. Using functions instead of global variables helps improve trace-ability of where a variable is used, prevents accidental writes by other modules, and helps create stub functions when we are writing unit tests.

• No global variables in the header file

  • You do not want to expose your data to anyone without an explicit API

  • You cannot declare instances of variables in a header file because each file including it will have its own instance leading to linking error with multiple definitions of the same name

  • Do not use extern in a header file

• No inline functions

  • Inline functions cannot be mocked for unit testing. When mocking a typical function we replace a functions definition with our own definition (this is done by compiling with a mock.c file that replaces of the real one). However we keep the header file with the function declarations because the code we're testing relies on this. Because inline functions are both declared and defined in the header file we can't replace the already existing definition and therefore are unable to create mocks for inline functions that exist in header files.

  • Inline functions are not picked up by Doxygen

  • Inline is just a recommendation for some compilers

  • Too many invocations to the inline function will bloat code, and actually make it run slower (due to cache misses)

• Private struct for all working variables

  • All variables required by a module shall be in a private struct that is defined inside the C source file. This makes it easy to see at a glance all the working variables of a module.

  • It makes the code easier to multi-thread in the future (because a pointer to the struct can be passed in to each of the functions)

  • The struct should be named like <layer>_<module>_data_s and the name of the variable should be <layer>_<module>_data. It is important to specify the layer in the name, only because we have both app_seats.c and io_seats.c in the code base and the working variables for each should be differentiated.

  • Example:

    • In app_seats.c, it would be called app_seats_data_s and the name of the variable would be app_seats_data

    • In io_seats.c, it would be called io_seats_data_s and the name of the variable would be io_seats_data

• No access from lower layers to higher layers

  • Horizontal or downward code access only.

  • Code in a particular layer can only call other code in that layer or in the layer that is immediately below it. So, for example, app_seats.c can call app_airbag.c because it is another app. It can also call dev_seats because dev is the layer immediately below app.

• No static variables inside functions (Does not apply to static const)

  • They are hard to test since they do not tear down correctly

  • They create cross-thread race conditions as it is a single instance that may be accessed and manipulated across multiple tasks (or threads).

  • Prefer to add any such memory within the main module struct instead

  • If it is indeed a single app then declare it global in the source file rather than inside of a function

  • Example:

Data Structure Guidelines

Strict types

When defining arrays, the use of hardcoded values and macros for array sizes must be avoided. Instead, the memory should be wrapped in a struct.

Do not:

Typedefs are generally preferred, but they can also lead to problems in this example:

Hence, the better approach is to wrap the packet type inside of a struct in order to make the types more strict:

Containerize Associated Data

Consider a case in which there is a pointer to memory, and size of the memory. The disadvantage of separate variables are:

• Variable names need to be accurate to describe the intent that memory_size is associated with the memory pointer

• Passing data from one function to another requires two parameters

Instead, opt to use a container:

Using sl_data_s in this examples consolidates related data, and avoids loosely coupled variables. This kind of concept is more natural in C++ as it has containers such as vector or list that have its related variables inside a single container, but in C, we have to take this step ourselves. ISO CPP Guidelines uses span container that would contain the array and its size for example.

Software Layering

Whitespace & Code Format

Newer languages like Golang have whitespace rules, such as the brace positions built into the language. We chose to implement same idea by using clang-format, which auto formats the source code according to pre-set rules such that we focus our code reviews on the actual matter, and not the code format.

Sibros has clang-format set up to auto-format your code during git commit. Google C++ code whitespace formatting rules are used to format code during a post-commit hook. The only exception to the Google rules is that we use 120 character limit for lines instead of 80 characters.

Layers

There are two primary objectives for layering the code:

• You should be able to look at any piece of code and assess what it does, where it lives and which other layers and functions it interacts with.

• Each code module should be in a namespace of its own layer that indicates its intent and describes where it fits in the hierarchy.

Shown below are two high level diagrams of the system/software architecture, as well as an explanation for each layer.

System Architecture

Open

Software Architecture

Open

The layers are:

• app_ : Application Layer

  • Application layer is the highest level SW layer, where the high level system behaviors and features are implemented.

• dev_ : Device Communication Layer

  • The device layer provides access to physical devices that may be connected and controlled by an ECU. The API for modules in this layer provides the access and control of these devices, abstracting away the ECU HW that is used to physically control and communicate with them. Therefore, the module using the API can be abstracted from the ECU and become portable to any ECU that has implemented the device layer module. This layer is also abstracting away the physical location of the device being controlled, such that the device could actually be connected via a communication bus and commanded remotely. Therefore, this layer acts as a communication abstraction layer to other devices and ECUs.

• ecu_ : ECU and Complex IO Layer

  • Provides access to interface with ECU board level HW. These interfaces abstract away the HW details, such as which particular circuits or ICs are on the board, and instead provide a common interface layer to control the functions these circuits provide. This allows higher level control of ECU pins, abstracting away the details of the circuits driving those pins. This allows for a common interface to control an ECU, regardless of which variant of ECU hardware is actually being operated on.

• mcu_ : Microcontroller and Low Level IO Layer

  • Used to interface with microcontrollers, allowing control of the micro’s pins, and operating states, without having to directly access the micro’s peripherals or CPU cores. Modules at this layer implement basic drivers that utilize a particular microcontroller’s peripherals and CPU core. This layer abstracts away the microcontroller, allowing for a common interface layer across different microcontrollers.

• perif_: Microcontroller Peripheral and CPU Core Layer

  • Used to interface with peripherals and CPU cores. This gives the ability to control and access a peripheral, without having to directly access registers or peripheral memory. The interfaces for a given module provide abstraction to the specifics of the peripheral, allowing for the same interfaces to be used across multiple variations of the peripheral. Variations might include the number of channels or number of instances of that peripheral. A peripheral may be used in various microcontroller families, so having an interface layer to the peripheral allows for reuse across microcontrollers.

• os_ : OS Abstraction Layer (including RTOS)

  • The OS layer provides a common abstraction layer from the underlying operating system (OS). This allows the code to be portable to any ECU, regardless of the underlying OS, even a bare-metal system.

• sl_ : Standalone and Utility Library Layer

  • Code modules in this layer are implemented as pure functions or utility macros, or define standard types. These modules have no internal state, do not have external dependencies, and do not have run tasks. They are mathematical libraries, conversion functions, or standardized type definitions. The only external dependencies allowed would be other standard utility library modules.

    • Example 1: sl_crc32 only operates on data passed as parameters to it's API. It operates on the parameters and returns a result. It does internally store any data, and therefore contains no internal state or memory.

    • Example 2: sl_uds_server is a complex module that depends on other SL modules, but still does not store any data internally. All information is passed via parameters to the API. Any static information is stored within the application that creates an instance of sl_uds_server, and simply uses the sl_uds_server API to opperate on that instance.

Double Underscore

Double underscores are used to quickly distinguish what layer and module something belongs to. All functions, variables, structs, enums and unions in a file should match the file name. Please look at the diagram below and the explanations that follow afterwards.

Observe the following:

• Each variant contains dev_file_io.c in their folder. This implements all the public functions defined in the main header file dev_file_io.h. Additionally, if extra public functions need to be defined for this particular variant, they would live in a new file called dev_file_io_posix.c/.h and all code in those files would start with the name dev_file_io_posix__.

• When integrating dev_file_io into a project, the integrator will configure which variant to use. So, if the project is a POSIX based system, then they would integrate the posix variant.

Include Guard

Sibros initially went with #pragma once as an include guard. The thought process was that "most" compilers support this compiler pre-processor tag. Sibros, however, supports multitude of OEMs and a diverse set of compilers and we eventually ran into a Tasking compiler that would not recognize the pragma once and had to revise the strategy. Therefore, the include guard standard is:

The reason to tag with SIBROS__ is that it guarantees that we will not run into third party code base that happens to use the same name for the include guard, and therefore have to make the include guard unique.

For C++ code base, follow the #pragma once for header files. For C++ code base we deviated from the C include guard because C++ codebase is dominantly compiled in GCC or similar modern compilers and C++ code base is usually not compiled by diverse set of embedded/C compilers. We decided to use this after weighing in on the benefits and the side-effects. The Wikipedia definition is on the harsh side against its use, but there are benefits to avoiding the cryptic #ifndef FOLDER_FOO_HEADER_H__ style.

Reasons in favor:

• Avoids boiler-plate code

• Avoids creating a unique #ifdef guard

Reasons against:

• A small fraction of the compilers do not support this

C++ extern

The following block of code is required so that C++ code can use C header files and call the C code. Please note that this is only required in header files and not required in source files.

Order of Includes

Follow the examples below, but note that the comments in between include blocks should be used to separate different groups of includes otherwise clang-format will re-order the includes in alphabetical order. The order of include is important because during unit-tests, we hijack the meaning of static keyword and unfortunately the order of includes can lead to trouble during unit-tests. The Unit-Test article further discusses why we hijacked static instead of inventing a new keyword, such as STATIC.

• Standard Includes consist of standard C libs, and should be included with < > instead of " "

• External Includes consist of any module that is not the one we are writing, regardless of whether it is in-house or third-party

• Module Includes consist of all files that are specific to the module we are writing

Header File

• A module's header file should not include sl_unit_test_facilitator.h

Private Header File

Not all code modules have to have the private header file. Private function declarations for a code module should go in the *_private.h file. This is because the unit tests may need to access the static functions to perform detailed tests.

The order of includes is just like an ordinary header file, except that we include sl_unit_test_facilitator.h last to hijack the static keyword such that it has no effect during unit-tests. After this file is included, and only during the Unit-Tests, it replaces the meaning of static with <blank> such that the code's private functions are not private and this facilitates Unit-Testing.

Source File

• The source file's header, additionally handles the logic of determining if the module's feature flag is defined. Consequently, the header is included first so that we can inherit the module's feature flag as early as possible in the source file

• The source file's private header should be included last

  • The desire is to re-define static as late as possible

• If you have the *_private.h version of your header file, then that includes sl_unit_test_facilitator.h already, so no inclusion is necessary explicitly.

• If you do not have *_private.h then:

  • Include sl_unit_test_facilitator.h in place of *_private.h

Unit Test file

Naming Convention

General Rules

• Snake Case

  • After some research, we decided to go with snake_case or the underscore convention, primarily due to the following reasons:

  • Snake case does not suffer from:

    • Differentiating Abbreviations tcpIpOpen

    • I hate Camel Case

  • Naming convention inside code can match the file names

    • Use myCodeModule.c creates issues with diverse build systems (Windows vs. Linux)

  • Therefore, use snake_case with no capital letters for all code, file and folder names

    • Do not use camelCase

    • Do not use PascalCase

    • Do not use Hungarian notation (like pMyString for pointers, 'bSwitch' for booleans)

    • Do not mix dashes with underscores. e.g. pre_integration_plan is OK; pre-integration_plan is not OK.

• Names of basic types and code modules

  • Use UPPER_CASE_SNAKE_CASE for

    • Global and static global Constants (local variables that are const can be lowercase to differentiate between a global const variable vs. local const variable)

      • Upon further discussion, it was decided to use lower_case_snake_case for all variables, including constants. This is because we don’t want global const struct instances to be UPPER_CASE_SNAKE_CASE, and we don’t want to make a confusing exception between global const scalar types being UPPER_CASE_SNAKE_CASE and global const aggregate types being lower_case_snake_case. The original benefit of using UPPER_CASE_SNAKE_CASE for constants was to easily identify which variables are constant. However, all of our global constants are file scope, so const variables can easily be identified as being constant by viewing their declaration in the same file.

    • Macros

    • Enumeration values

  • Code Module

    • Precede with layer name

    • Create a header file, source file, and its corresponding unit-test file

    • If there are private enums, typedefs, or functions, then also create the private module

    • Example: app_my_module.h, app_my_module.c, app_my_module_private.h, test_app_my_module.c

• All code, file names and folder names in lowercase

  • All acronyms shall be lowercase, just like all other names, whether they are part of functions, variables, or any other types

  • The reason for this is to keep naming consistent with the file naming convention, which uses all lowercase

  • Example: your module for Unified Diagnostic Services shall be abbreviated as uds and not UDS

• Use American English spelling and not British English spelling.

  • Example: behavior instead of behaviour

  • Because Sibros started in America

• No single letter variables, not even in for loops.

  • Instead of i and j, use index or something else that describes what you are looping over

  • Because you should have more meaning to the variable

• No Hungarian notation for variable names

  • Because variables change meaning, or the notation doesn't convey any useful information. For example, do not create boolean variables starting with b like b_door_open and pointers with p like p_battery_data.

  • Because this notation existed in the old days with lack of good IDEs

• No global variables that can be accessed using an extern

  • You should make them static in your source code, to forbid extern access

  • If you want to have access to a variable, it should be returned from a public access function

  • Because we don’t want peek and poke code

• Use the verb-noun method for naming routines that perform some operation on a given object, such as:

  • app_battery__calculate_soc()

  • This allows for the reader of the code to be able to easily understand the action that the function is taking.

Variable Names

• Use standard capitalization rules for measurement variables even if it breaks acronym and abbreviation naming rules. This is done to reduce confusion of someone reading the code and expecting certain standard capitalization for measurement variables (i.e. meter = m, watt = W, pascal = Pa).

  • speed_mph

  • time_left_ms

• Variables should be singular, and arrays should be plural.

  • Example:

Limit Abbreviations

• Examples:

  • Use packet instead of pkt

  • Use calculate instead of calc

  • Use index instead of idx

• If abbreviations are used, be consistent in their use. An abbreviation should have only one meaning and likewise, each abbreviated word should have only one abbreviation.

• It is okay to use abbreviations for standard unit types (kg, ms, lbs)

• Here are some specific variables name that are not allowed:

  • Do not abbreviate minimum as min because what if someone later abbreviates minute as min?

  • No variables named temp because temp can mean temporary, and can mean temperature also.

No Redundant Names

• In object-oriented languages, it is redundant to include class names in the name of class properties, such as book.book_title. Instead, use book.title

• This applies to things like structs, unions and enums.

• Avoid repeating words like queue: void sl_queue__init_queue(...);

Avoid the following because it has the object name repeated too many times:

Avoid repeating the module name in member variables of a struct.

Types

We should be able to decipher the type looking at the name of the type. Therefore, follow the following naming convention for new types.

• Enums: typedef enum enum_e;

• Structs: typedef struct struct_s;

• Unions: typedef union union_u;

• Type: typedef old_type new_type_t;

• Function Pointer: typedef void (*func_ptr_f)(void);

Avoid obscuring a type, for example: typedef char hostname[32];. This makes the code unsafe because an aribtrary char array can get past the compiler checks:

You can make this code safer by using a data structure which will avoid the oops() function above.

Use strong types

Avoid creating types that can fail basic sanity checks. Here is an example:

Instead, opt for better types:

Also, avoid macros that can make it through type checks. C may be limited (no C++ templates), but you rather be safe than sorry.

Preferred Types

Use <stdint.h>

• Avoid using int or long because they may not port across CPUs with different bit-width

  • Instead use explicit uint32_t or int64_t etc.

• Do not define your own bool (examples: BOOL, _bool, bool_t), use #include <stdbool.h> instead

Use size_t

• Use size_t for any "size types" of unsigned numbers

  • Use for indexes, and container sizes, much like the C++ library

• Do not use size_t when you want the size to be explicit

  • Example: uint32_t for a 32-bit address of SPI flash

• Use ssize_t for any signed types

Functions

• Public functions should be separated from the layer and the module name with two underscores.

  • Pattern: <layer>_<module>__<api name>();

    • <layer>_<module> in the function name must match the file name

  • Examples:

  • app_traction_control__init()

  • app_lights__disable()

  • app_battery__calculate_soc()

• Private functions

  • Shall have layer and module in the name.

  • Shall have the word private in it

    • This improves code readability because in a large code module, it is easier to differentiate the function invocation internal or external to the code module

  • Pattern: <layer>_<module>__private_<api name>();

    • <layer>_<module> in the function name must match the file name

  • Examples:

    • void sl_uds_client__private_func(void);

    • app_battery__private_update_soc(void)

• Test functions

  • Pattern: test_<layer>_<module>__<api name>__<test description>();

    • <layer>_<module> in the function name must match the file name

  • Examples:

    • test_app_traction_control__init__initalization_fails()

    • test_app_lights__disable__parameter_is_null()

    • test_app_battery__calculate_soc__invalid_soc()

• Callback Functions

  • Callback functions should always contain an argument as this allows the caller to provide context to the function. This allows a single callback function to be used for multiple cases.If the argument is of void* then ensure that the argument is not a const.

  • Incorrect Method:

  • Correct Method:

Enums

• Typedef Name Pattern: <layer>_<module>__<enum purpose>_e

  • <layer>_<module> in the enum name must match the file name, unless the file is in format <layer>_<module>_types, eg. (mcu_can_types.h has mcu_can__hw_mailbox_e)

  • Example: app_battery__status_codes_e;

• Tag Name Pattern: <layer>_<module>__<enum purpose>_tag

  • <layer>_<module> in the enum name must match the file name, unless the file is in format <layer>_<module>_types

  • Example: app_battery__status_codes_tag

  • Tag is optional, but must follow this convention if included

• Start the first enum value at zero

  • Prefer to use an unspecified value for the first enum

• (Optional) Define a count value only for iterable lists

  • This can be used for sizing arrays of structs, where each element is identified by an enum.

• Put a comma at the last enum value

  • Creates less diff during code review

  • Allows you to move enums without a compiler error

• Values inside the enum table should use the specific name of the enum

  • Example: <LAYER>_<MODULE_NAME>__<DESCRIPTION>

Example

Example of a list

Structs

• Typedef Name Pattern: <layer>_<module>__<struct purpose>_s or <layer>_<module>_s

  • <layer>_<module> in the struct name must match the file name, unless the file is in format <layer>_<module>_types, eg. (mcu_can_types.h has mcu_can__hw_handle_s)

  • Example: hal_spi__data_s;

  • If the struct is the file/module name itself, we can omit the purpose.
    sl_ecu_info_s;

• Tag Pattern: <layer>_<module>__<struct purpose>_tag

  • <layer>_<module> in the struct name must match the file name, unless the file is in format <layer>_<module>_types

  • Example: hal_spi__data_tag

  • If the struct is the file/module name itself, we can omit the purpose.
    sl_ecu_info_s;

  • Tag is optional, but must follow this convention if included

• Struct member variables do not need layer or module name.

• Initialization of structs:

  • Prefer trivial types at the beginning of your struct. It is then easier to instantiate it like so: struct_s s = { 0 };

  • If you have a nested struct or union, and you want zero initialization, then putting trivial types at the beginning will result in safer code because you can initialize the first member and omit the other members to zero initialize themselves

    In the case above, the following notation will fail:

    Reorganizing the struct to put the union last like mentioned above will fix this issue.

    • The clang compiler does not like C++ : initialization, such as struct_s s = { foo: 0, bar: 1}, so do not use this method.

    • Not using dot initialization is dangerous because if someone changes the order of the members, it will create buggy code.

    • However dot initialization or C++ ':' colon initialization does not work well for char arrays due to the following compiler error: 'C99 designator outside aggregate initializer'. Do to this initialization of a char array in a struct should be done like so:

• Private data in structs

  • If there is any data that you want to designate as private in a struct then simply store it in a internal struct named private_data:

Unions

• Typedef Name Pattern: <layer>_<module>__purpose_u

  • <layer>_<module> in the union name must match the file name, unless the file is in format <layer>_<module>_types

  • Example: app_door__config_u;

• Tag Pattern: <layer>_<module>__purpose_tag

  • <layer>_<module> in the union name must match the file name, unless the file is in format <layer>_<module>_types

  • Example: app_door__config_tag

  • Tag is optional, but must follow this convention if included

• Otherwise same as struct rules

Function Pointers

• typedef Function Pointers if you use it more than once.

• Typedef function pointer pattern: <layer>_<module>__<function name>_f

  • <layer>_<module> in the function pointer name must match the file name

  • Example: typedef void (*hal_pwm__callback_f)(const uint8_t *data);

While Loops

• do while loops should not be used to keep consistency across modules. Use a while loop instead.

Macros

• DO NOT USE MACROS

  • Each macro use needs to be discussed with a team member before it is added

• Avoid macros at all costs.

  • You must prove that a function is unable to achieve something before you create a function-like macro

• Most use cases of #defines can alternatively use consts, which can be defined by library users in external modules.

Better alternatives to #defines are discussed below:

• Use const for compiler switches

  • Const supports unit testing whereas macros do not since they can not be modified in a test.

  • To test a macro #if () the code would have to be rewritten multiple times to test the alternative cases.

  • If a const variable is used, the variable can be altered.

• Use const in place of #defines because it binds a type to the variable.

  • There may be a genuine reason where const would not work, so treat this as an exception. For example:

  • Note that even for the case above, the following is preferred if the const is only used for the array size:

• Functions

  • Only define function-like macros at sl_function_macros.h

  • You will get scrutinized if you wish to add more macros to this file

  • Prefer to use real functions, and do not optimize unless you perform timing analysis and can prove a macro is better than a function for a specific use case

  • It may be okay to create macros as replacements of C++ template types, such as MIN_OF()

Macro guidelines

• Macros should always be UPPER CASE

  • #define MY_MACRO_ALL_CAPS_WITH_UNDERSCORES (42)

• Macros shall be parenthesized in their definition to prevent someone from accidentally mis-using it. For example:

  • #define MIN_OF (a, b) (a < b) ? a : b // THIS IS BAD

  • #define MIN_OF (a, b) (((a) < (b)) ? (a) : (b)) // THIS IS GOOD

• if macro statements should always follow the pattern: #if (...)

  • Example with multiple conditions: #if ((...) && (...))

• Avoid using #ifdef MACRO when #if MACRO can be used

  • Example:

    • #define MACRO 0

    • #if MACRO // this is clear

    • #ifdef MACRO // this is not clear because it is not dependent on the actual value of MACRO

• Macro invocations do not need to be surrounded in parentheses.

  • Example:

    • #if FEATURE_UDS // This is perfectly fine

    • #if (FEATURE_WINDOWS) // This isn't required because the macro itself should be written defensively so that it doesn't cause a problem if those parentheses are missing

Macros as compile-time switches

Macros used for compile-time switching should be treated as boolean values. This means that a macro should either be defined as 0 to disable the feature or 1 to enable it.

Reference the following code of an arbitrary file, such as socket_proxy.c. Note that you want to avoid:

• #ifdef because if the user defines the macro as 0, #ifdef will still be true as the macro is defined.

  • You want to switch on #if FEATURE !=0 rather than #ifdef FEATURE

• Compare values of the #if explicitly

Example: socket_proxy.c:

Other Guidelines

String Literals in Print Statements

When using printf(const char *format, ...) or its variants, string literals should be used for the format parameter as opposed to a char * variable. The reason is because the compile can check the string literal’s % symbols, and match them against the number of arguments passed into printf() variants. The consequence of not matching the parameters with % symbols is stack corruption, hence, the compiler checks are vital for this use-case.

Code Templates

The empty lines between includes, and their order is very important. The clang-format will re-order #includes if they are not ordered correctly.

The templates follow the Order of Includes and we do not duplicate the reasons here.

We use these templates with the d3vnu1l.template-generator-vscode plugin for VSCode, hence the macros enclosed within {...} seen below.

Header File

Private Header File

Source File

Test File