aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRonald Cron <ronald.cron@arm.com>2019-06-28 13:10:09 +0200
committerronald-cron-arm <39518861+ronald-cron-arm@users.noreply.github.com>2019-07-23 11:24:29 +0200
commit5d05035a9a0f6f5a409e3acfb55e1009f2b7cc58 (patch)
tree5cef8e12abb85f4a714ad5e778c2c8b623c2d085
parent56eb6b77b445c1e8d2d7be3652c8d3fb808db11a (diff)
Improve coding style and rules
Change-Id: I04745f89d1f25f36ab1d2c0df235c1bb839660c3 Signed-off-by: Ronald Cron <ronald.cron@arm.com>
-rw-r--r--doc/code_rules.md102
-rw-r--r--doc/code_style.md29
2 files changed, 119 insertions, 12 deletions
diff --git a/doc/code_rules.md b/doc/code_rules.md
index 8dd9dfaa..09318f8a 100644
--- a/doc/code_rules.md
+++ b/doc/code_rules.md
@@ -1,8 +1,52 @@
Coding Rules
============
-To maintain consistency within the SCP/MCP software source code a series of
-rules and guidelines have been created; these form the project's coding rules.
+To maintain consistency within the SCP/MCP software source code and to reduce
+the risk of bugs, a series of rules and guidelines have been created; these form
+the project's coding rules.
+
+General Considerations
+----------------------
+The software follows the ISO/IEC 9899:2011 standard (C11). This is enforced
+through the use of compilation flags and all warnings being considered as
+errors. Compilation flags are also used to avoid constructs usually considered
+questionable, and that are easy to avoid.
+
+For robustness and reliability reasons, dynamic memory allocation is allocate-
+only and must be done through the facilities provided by the firmware framework.
+The intent is for memory allocations to be done during the pre-runtime phase or
+early in the runtime phase based on configuration data or hardware detection.
+Allocated memory cannot be freed or reallocated.
+
+The framework provides an optional multi-threading runtime, contingent on a
+CMSIS-RTOS-based RTOS. Interaction with the RTOS happens exclusively through the
+framework.
+
+C Standard Library
+------------------
+When developing a module, only the following headers of the C standard library
+are allowed to be included:
+`<limits.h>`, `<stdarg.h>`, `<stdbool.h>`, `<stddef.h>`, `<stdint.h>`,
+`<stdio.h>`, `<stdlib.h>` and `<string.h>`.
+
+Concerning the library functions defined in `<stdio.h>`, only `snprintf()` may
+be used.
+
+Concerning the library functions defined in `<stdlib.h>`, only `bsearch()`,
+`qsort()`, `abs()` and `rand()` may be used.
+
+Concerning the library functions defined in `<string.h>`, `strcat()` and
+`strcpy()` cannot be used. Use `strncat()` and `strncpy()` instead.
+
+If not already defined by another standard library header, include `<stddef.h>`
+and not `<stdlib.h>` to define `size_t`.
+
+Additionally, the framework wraps the following standard library header files:
+`<stdalign.h>`, `<stdnoreturn.h>`, `<assert.h>` and `<errno.h>`. These header
+files must not be directly included in module code. This is because certain
+compilers, while themselves C11-compliant, do not provide a full C11 standard
+library implementation. In this situation, the framework provides a custom
+implementation through these headers.
Header Files
------------
@@ -31,11 +75,13 @@ includes.
Types
-----
-Import "stdint.h" (part of the C Standard Library) for exact-width integer types
-(uint8_t, uint16_t, etc). These types can be used wherever the width of an
-integer needs to be specified explicitly.
+Import `<stdint.h>` (part of the C Standard Library) for exact-width integer
+types (`uint8_t`, `uint16_t`, etc). These types can be used wherever the width
+of an integer needs to be specified explicitly.
-Import "stdbool.h" (also part of the C Standard Library) whenever a "boolean"
+Use `uintptr_t` to handle addresses as integers.
+
+Import `<stdbool.h>` (also part of the C Standard Library) whenever a "boolean"
type is needed.
Avoid defining custom types with the "typedef" keyword where possible.
@@ -57,6 +103,27 @@ unsigned int counter;
size = sizeof(counter);
\endcode
+Use the `const` type qualifier as appropriate to specify values that cannot be
+modified. Quick reminder:
+
+- `const xyz_t object`, `object` is an object of type xyz_t which value cannot
+be modified.
+- `const xyz_t *ptr` or `xyz_t const *ptr`, `ptr` is a pointer to a constant
+object of type xyz_t. The value of the object cannot be modified, the value of
+the pointer can be modified.
+- `xyz_t * const ptr`, `ptr` is a constant pointer to an object of type xyz_t.
+The value of the object can be modified, the value of the pointer cannot be
+modified.
+- `const xyz_t * const ptr` or `xyz_t const * const ptr`, `ptr` is a constant
+pointer to a constant object of type xyz_t. The value of the object and the
+pointer cannot be modified.
+
+https://cdecl.org may help if in doubt.
+
+Static storage qualifier
+------------------------
+Declare functions and variables private to a C file as static.
+
Operator Precedence
-------------------
Do not rely on the implicit precedence and associativity of C operators. Use
@@ -127,17 +194,32 @@ struct clock clock_cpu = {
};
\endcode
+Integers
+--------
+To mitigate the risk of integer wrap-around, conversion or truncation errors:
+
+- represent integer values that should never be negative with the `unsigned`
+type that you expect to hold all possible values.
+
+- represent integer values that may be negative with the `signed` type that you
+expect to hold all possible values.
+
+- when taking untrusted integer input, ensure you check them against the lower
+and upper bound of the integer type you store them in.
+
Device Register Definitions
---------------------------
The format for structures representing memory-mapped device registers is
standardized.
-- The file containing the device structure must include stdint.h to gain access
-to the uintxx_t and UINTxx_C definitions.
-- The file containing the device structure must include fwk_macros.h to gain
-access to the FWK_R, FWK_W and FWK_RW macros.
+- The file containing the device structure must include `<stdint.h>` to gain
+access to the uintxx_t and UINTxx_C definitions.
+- The file containing the device structure must include `<fwk_macros.h>` to
+gain access to the FWK_R, FWK_W and FWK_RW macros.
- All non-reserved structure fields must be prefixed with one of the above
macros, defining the read/write access level.
+- Avoid C structure bit-fields when representing hardware registers - how
+bit-fields are represented in memory is implementation-defined.
- Bit definitions should be declared via UINTxx_C macros.
- Bit definitions must be prefixed by the register name it relates to. If bit
definitions apply to multiple registers, then the name must be as common as
diff --git a/doc/code_style.md b/doc/code_style.md
index 946254fe..a7e20afc 100644
--- a/doc/code_style.md
+++ b/doc/code_style.md
@@ -25,7 +25,25 @@ Avoid using:
Functions, macros, types and defines must have the "fwk_" prefix (upper case for
macros and defines) to identify framework API.
-It is fine and encouraged to use a variable named "i" (index) for loops.
+Non-static names should be prefixed with the name of their translation unit to
+avoid name collisions.
+
+It is acceptable to use the following common placeholder names for loop indices:
+ - `i`
+ - `j`
+ - `k`
+
+`xyz_idx` names are commonly used for indices that live longer than a single
+loop.
+
+License
+-------
+All files must begin with a license header of the following form:
+
+Arm SCP/MCP Software
+Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
+
+SPDX-License-Identifier: BSD-3-Clause
Inclusions
----------
@@ -37,6 +55,12 @@ Header file inclusions should follow a consistent sequence, defined as:
For each group, order the individual headers alphabetically.
+Header files (`.h` files) should include the headers needed for them to compile
+and only these ones.
+
+Translation units (`.c` files) should not rely on indirect inclusion to provide
+names they have otherwise not included themselves.
+
Indentation and Scope
---------------------
Indentation is made of spaces, 4 characters long with each line being at most 80
@@ -230,7 +254,8 @@ The project APIs are documented using Doxygen comments.
It is mandatory to document every API exposed to other elements of the project.
By default, the provided Doxygen configuration omits undocumented elements from
-the compiled documentation.
+the compiled documentation. APIs are documented in the header files exposing
+them.
At a minimum:
- All functions and structures must have at least a "\brief" tag.