From e63f5d129fadf520f42110d9a16c4192cba48784 Mon Sep 17 00:00:00 2001 From: Paul Beesley Date: Thu, 16 May 2019 13:33:18 +0100 Subject: doc: Split and expand coding style documentation This patch expands the coding style documentation, splitting it into two documents: the core style rules and extended guidelines. Note that it does not redefine or change the coding style (aside from section 4.6.2) - generally, it is only documenting the existing style in more detail. The aim is for the coding style to be more readable and, in turn, for it to be followed by more people. We can use this as a more concrete reference when discussing the accepted style with external contributors. Change-Id: I87405ace9a879d7f81e6b0b91b93ca69535e50ff Signed-off-by: Paul Beesley Signed-off-by: Petre-Ionut Tudor --- docs/process/coding-guidelines.rst | 515 ++++++++++++++----------------------- 1 file changed, 190 insertions(+), 325 deletions(-) (limited to 'docs/process/coding-guidelines.rst') diff --git a/docs/process/coding-guidelines.rst b/docs/process/coding-guidelines.rst index cb8b89245..f7d53a97e 100644 --- a/docs/process/coding-guidelines.rst +++ b/docs/process/coding-guidelines.rst @@ -1,232 +1,126 @@ -Coding Style & Guidelines -========================= +Coding Guidelines +================= -The following sections contain TF coding guidelines. They are continually -evolving and should not be considered "set in stone". Feel free to question them -and provide feedback. +This document provides some additional guidelines to consider when writing +|TF-A| code. These are not intended to be strictly-enforced rules like the +contents of the :ref:`Coding Style`. -Some of the guidelines may also apply to other codebases. +Automatic Editor Configuration +------------------------------ -.. note:: - The existing TF codebase does not necessarily comply with all the - below guidelines but the intent is for it to do so eventually. - -Checkpatch overrides --------------------- - -Some checkpatch warnings in the TF codebase are deliberately ignored. These -include: - -- ``**WARNING: line over 80 characters**``: Although the codebase should - generally conform to the 80 character limit this is overly restrictive in some - cases. - -- ``**WARNING: Use of volatile is usually wrong``: see - `Why the “volatile” type class should not be used`_ . Although this document - contains some very useful information, there are several legitimate uses of - the volatile keyword within the TF codebase. - -Headers and inclusion ---------------------- - -Header guards -^^^^^^^^^^^^^ - -For a header file called "some_driver.h" the style used by the Trusted Firmware -is: - -.. code:: c - - #ifndef SOME_DRIVER_H - #define SOME_DRIVER_H - -
- - #endif /* SOME_DRIVER_H */ +Many of the rules given below (such as indentation size, use of tabs, and +newlines) can be set automatically using the `EditorConfig`_ configuration file +in the root of the repository: ``.editorconfig``. With a supported editor, the +rules set out in this file can be automatically applied when you are editing +files in the |TF-A| repository. -Include statement ordering -^^^^^^^^^^^^^^^^^^^^^^^^^^ - -All header files that are included by a source file must use the following, -grouped ordering. This is to improve readability (by making it easier to quickly -read through the list of headers) and maintainability. - -#. *System* includes: Header files from the standard *C* library, such as - ``stddef.h`` and ``string.h``. - -#. *Project* includes: Header files under the ``include/`` directory within TF - are *project* includes. - -#. *Platform* includes: Header files relating to a single, specific platform, - and which are located under the ``plat/`` directory within TF, - are *platform* includes. - -Within each group, ``#include`` statements must be in alphabetical order, -taking both the file and directory names into account. - -Groups must be separated by a single blank line for clarity. - -The example below illustrates the ordering rules using some contrived header -file names; this type of name reuse should be otherwise avoided. - -.. code:: c - - #include - - #include - #include - #include - #include - - #include "./a_header.h" - -Include statement variants -^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Two variants of the ``#include`` directive are acceptable in the TF codebase. -Correct use of the two styles improves readability by suggesting the location -of the included header and reducing ambiguity in cases where generic and -platform-specific headers share a name. - -For header files that are in the same directory as the source file that is -including them, use the ``"..."`` variant. - -For header files that are **not** in the same directory as the source file that -is including them, use the ``<...>`` variant. - -Example (bl1_fwu.c): - -.. code:: c +Several editors include built-in support for EditorConfig files, and many others +support its functionality through plugins. - #include - #include - #include +Use of the EditorConfig file is suggested but is not required. - #include "bl1_private.h" -Platform include paths -^^^^^^^^^^^^^^^^^^^^^^ - -Platforms are allowed to add more include paths to be passed to the compiler. -The ``PLAT_INCLUDES`` variable is used for this purpose. This is needed in -particular for the file ``platform_def.h``. - -Example: - -.. code:: c - - PLAT_INCLUDES += -Iinclude/plat/myplat/include +Automatic Compliance Checking +----------------------------- -Types and typedefs ------------------- +To assist with coding style compliance, the project Makefile contains two +targets which both utilise the `checkpatch.pl` script that ships with the Linux +source tree. The project also defines certain *checkpatch* options in the +``.checkpatch.conf`` file in the top-level directory. -Use of built-in *C* and *libc* data types -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +.. note:: + Checkpatch errors will gate upstream merging of pull requests. + Checkpatch warnings will not gate merging but should be reviewed and fixed if + possible. -The TF codebase should be kept as portable as possible, especially since both -64-bit and 32-bit platforms are supported. To help with this, the following data -type usage guidelines should be followed: +To check the entire source tree, you must first download copies of +``checkpatch.pl``, ``spelling.txt`` and ``const_structs.checkpatch`` available +in the `Linux master tree`_ *scripts* directory, then set the ``CHECKPATCH`` +environment variable to point to ``checkpatch.pl`` (with the other 2 files in +the same directory) and build the `checkcodebase` target: -- Where possible, use the built-in *C* data types for variable storage (for - example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99* - types. Most code is typically only concerned with the minimum size of the - data stored, which the built-in *C* types guarantee. - -- Avoid using the exact-size standard *C99* types in general (for example, - ``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the - compiler from making optimizations. There are legitimate uses for them, - for example to represent data of a known structure. When using them in struct - definitions, consider how padding in the struct will work across architectures. - For example, extra padding may be introduced in AArch32 systems if a struct - member crosses a 32-bit boundary. +.. code:: shell -- Use ``int`` as the default integer type - it's likely to be the fastest on all - systems. Also this can be assumed to be 32-bit as a consequence of the - `Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call - Standard for the Arm 64-bit Architecture`_ . + make CHECKPATCH=/linux/scripts/checkpatch.pl checkcodebase -- Avoid use of ``short`` as this may end up being slower than ``int`` in some - systems. If a variable must be exactly 16-bit, use ``int16_t`` or - ``uint16_t``. +To just check the style on the files that differ between your local branch and +the remote master, use: -- Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given - that `int` is 32-bit on Arm platforms, there is no use for it. For integers of - at least 64-bit, use ``long long``. +.. code:: shell -- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data. + make CHECKPATCH=/linux/scripts/checkpatch.pl checkpatch -- Use ``unsigned`` for integers that can never be negative (counts, - indices, sizes, etc). TF intends to comply with MISRA "essential type" coding - rules (10.X), where signed and unsigned types are considered different - essential types. Choosing the correct type will aid this. MISRA static - analysers will pick up any implicit signed/unsigned conversions that may lead - to unexpected behaviour. +If you wish to check your patch against something other than the remote master, +set the ``BASE_COMMIT`` variable to your desired branch. By default, +``BASE_COMMIT`` is set to ``origin/master``. -- For pointer types: +Ignored Checkpatch Warnings +^^^^^^^^^^^^^^^^^^^^^^^^^^^ - - If an argument in a function declaration is pointing to a known type then - simply use a pointer to that type (for example: ``struct my_struct *``). +Some checkpatch warnings in the TF codebase are deliberately ignored. These +include: - - If a variable (including an argument in a function declaration) is pointing - to a general, memory-mapped address, an array of pointers or another - structure that is likely to require pointer arithmetic then use - ``uintptr_t``. This will reduce the amount of casting required in the code. - Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it - may work but is less portable. +- ``**WARNING: line over 80 characters**``: Although the codebase should + generally conform to the 80 character limit this is overly restrictive in some + cases. - - For other pointer arguments in a function declaration, use ``void *``. This - includes pointers to types that are abstracted away from the known API and - pointers to arbitrary data. This allows the calling function to pass a - pointer argument to the function without any explicit casting (the cast to - ``void *`` is implicit). The function implementation can then do the - appropriate casting to a specific type. +- ``**WARNING: Use of volatile is usually wrong``: see + `Why the “volatile” type class should not be used`_ . Although this document + contains some very useful information, there are several legimate uses of the + volatile keyword within the TF codebase. - - Use ``ptrdiff_t`` to compare the difference between 2 pointers. +Performance considerations +-------------------------- -- Use ``size_t`` when storing the ``sizeof()`` something. +Avoid printf and use logging macros +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- Use ``ssize_t`` when returning the ``sizeof()`` something from a function that - can also return an error code; the signed type allows for a negative return - code in case of error. This practice should be used sparingly. +``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``) +which wrap ``tf_log`` and which allow the logging call to be compiled-out +depending on the ``make`` command. Use these macros to avoid print statements +being compiled unconditionally into the binary. -- Use ``u_register_t`` when it's important to store the contents of a register - in its native size (32-bit in AArch32 and 64-bit in AArch64). This is not a - standard *C99* type but is widely available in libc implementations, - including the FreeBSD version included with the TF codebase. Where possible, - cast the variable to a more appropriate type before interpreting the data. For - example, the following struct in ``ep_info.h`` could use this type to minimize - the storage required for the set of registers: +Each logging macro has a numerical log level: .. code:: c - typedef struct aapcs64_params { - u_register_t arg0; - u_register_t arg1; - u_register_t arg2; - u_register_t arg3; - u_register_t arg4; - u_register_t arg5; - u_register_t arg6; - u_register_t arg7; - } aapcs64_params_t; + #define LOG_LEVEL_NONE 0 + #define LOG_LEVEL_ERROR 10 + #define LOG_LEVEL_NOTICE 20 + #define LOG_LEVEL_WARNING 30 + #define LOG_LEVEL_INFO 40 + #define LOG_LEVEL_VERBOSE 50 -If some code wants to operate on ``arg0`` and knows that it represents a 32-bit -unsigned integer on all systems, cast it to ``unsigned int``. +By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will +be compiled into debug builds and all statements with a log level +``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be +overridden from the command line or by the platform makefile (although it may be +necessary to clean the build directory first). For example, to enable +``VERBOSE`` logging on FVP: -These guidelines should be updated if additional types are needed. +``make PLAT=fvp LOG_LEVEL=50 all`` -Avoid anonymous typedefs of structs/enums in headers -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Use const data where possible +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -For example, the following definition: +For example, the following code: .. code:: c - typedef struct { + struct my_struct { int arg1; int arg2; - } my_struct_t; + }; + void init(struct my_struct *ptr); + + void main(void) + { + struct my_struct x; + x.arg1 = 1; + x.arg2 = 2; + init(&x); + } is better written as: @@ -237,31 +131,18 @@ is better written as: int arg2; }; -This allows function declarations in other header files that depend on the -struct/enum to forward declare the struct/enum instead of including the -entire header: - -.. code:: c - - #include - void my_func(my_struct_t *arg); - -instead of: - -.. code:: c - - struct my_struct; - void my_func(struct my_struct *arg); - -Some TF definitions use both a struct/enum name **and** a typedef name. This -is discouraged for new definitions as it makes it difficult for TF to comply -with MISRA rule 8.3, which states that "All declarations of an object or -function shall use the same names and type qualifiers". + void init(const struct my_struct *ptr); -The Linux coding standards also discourage new typedefs and checkpatch emits -a warning for this. + void main(void) + { + const struct my_struct x = { 1, 2 }; + init(&x); + } -Existing typedefs will be retained for compatibility. +This allows the linker to put the data in a read-only data section instead of a +writeable data section, which may result in a smaller and faster binary. Note +that this may require dependent functions (``init()`` in the above example) to +have ``const`` arguments, assuming they don't need to modify the data. Libc functions that are banned or to be used with caution --------------------------------------------------------- @@ -410,14 +291,14 @@ error. This situation should be handled in one of the following ways: then emit an ``ERROR`` message and call the platform-specific function ``plat_error_handler()``. -Cases 1 and 2 are subtly different. A platform may implement ``plat_panic_handler`` -and ``plat_error_handler`` in the same way (for example, by waiting for a secure -watchdog to time-out or by invoking an interface on the platform's power -controller to reset the platform). However, ``plat_error_handler`` may take -additional action for some errors (for example, it may set a flag so the -platform resets into a different mode). Also, ``plat_panic_handler()`` may -implement additional debug functionality (for example, invoking a hardware -breakpoint). +Cases 1 and 2 are subtly different. A platform may implement +``plat_panic_handler`` and ``plat_error_handler`` in the same way (for example, +by waiting for a secure watchdog to time-out or by invoking an interface on the +platform's power controller to reset the platform). However, +``plat_error_handler`` may take additional action for some errors (for example, +it may set a flag so the platform resets into a different mode). Also, +``plat_panic_handler()`` may implement additional debug functionality (for +example, invoking a hardware breakpoint). Examples of unexpected unrecoverable errors: @@ -456,131 +337,115 @@ Examples: - Secure world is waiting for a hardware response that is critical for continued operation. -Security considerations ------------------------ - -Part of the security of a platform is handling errors correctly, as described in -the previous section. There are several other security considerations covered in -this section. - -Do not leak secrets to the normal world -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -The secure world **must not** leak secrets to the normal world, for example in -response to an SMC. - -Handling Denial of Service attacks -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Use of built-in *C* and *libc* data types +----------------------------------------- -The secure world **should never** crash or become unusable due to receiving too -many normal world requests (a *Denial of Service* or *DoS* attack). It should -have a mechanism for throttling or ignoring normal world requests. +The |TF-A| codebase should be kept as portable as possible, especially since +both 64-bit and 32-bit platforms are supported. To help with this, the following +data type usage guidelines should be followed: -Performance considerations --------------------------- +- Where possible, use the built-in *C* data types for variable storage (for + example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99* + types. Most code is typically only concerned with the minimum size of the + data stored, which the built-in *C* types guarantee. -Avoid printf and use logging macros -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Avoid using the exact-size standard *C99* types in general (for example, + ``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the + compiler from making optimizations. There are legitimate uses for them, + for example to represent data of a known structure. When using them in struct + definitions, consider how padding in the struct will work across architectures. + For example, extra padding may be introduced in |AArch32| systems if a struct + member crosses a 32-bit boundary. -``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``) -which wrap ``tf_log`` and which allow the logging call to be compiled-out -depending on the ``make`` command. Use these macros to avoid print statements -being compiled unconditionally into the binary. +- Use ``int`` as the default integer type - it's likely to be the fastest on all + systems. Also this can be assumed to be 32-bit as a consequence of the + `Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call + Standard for the Arm 64-bit Architecture`_ . -Each logging macro has a numerical log level: +- Avoid use of ``short`` as this may end up being slower than ``int`` in some + systems. If a variable must be exactly 16-bit, use ``int16_t`` or + ``uint16_t``. -.. code:: c +- Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given + that `int` is 32-bit on Arm platforms, there is no use for it. For integers of + at least 64-bit, use ``long long``. - #define LOG_LEVEL_NONE 0 - #define LOG_LEVEL_ERROR 10 - #define LOG_LEVEL_NOTICE 20 - #define LOG_LEVEL_WARNING 30 - #define LOG_LEVEL_INFO 40 - #define LOG_LEVEL_VERBOSE 50 +- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data. +- Use ``unsigned`` for integers that can never be negative (counts, + indices, sizes, etc). TF intends to comply with MISRA "essential type" coding + rules (10.X), where signed and unsigned types are considered different + essential types. Choosing the correct type will aid this. MISRA static + analysers will pick up any implicit signed/unsigned conversions that may lead + to unexpected behaviour. -By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will -be compiled into debug builds and all statements with a log level -``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be -overridden from the command line or by the platform makefile (although it may be -necessary to clean the build directory first). For example, to enable -``VERBOSE`` logging on FVP: +- For pointer types: -``make PLAT=fvp LOG_LEVEL=50 all`` + - If an argument in a function declaration is pointing to a known type then + simply use a pointer to that type (for example: ``struct my_struct *``). -Use const data where possible -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + - If a variable (including an argument in a function declaration) is pointing + to a general, memory-mapped address, an array of pointers or another + structure that is likely to require pointer arithmetic then use + ``uintptr_t``. This will reduce the amount of casting required in the code. + Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it + may work but is less portable. -For example, the following code: + - For other pointer arguments in a function declaration, use ``void *``. This + includes pointers to types that are abstracted away from the known API and + pointers to arbitrary data. This allows the calling function to pass a + pointer argument to the function without any explicit casting (the cast to + ``void *`` is implicit). The function implementation can then do the + appropriate casting to a specific type. -.. code:: c + - Avoid pointer arithmetic generally (as this violates MISRA C 2012 rule + 18.4) and especially on void pointers (as this is only supported via + language extensions and is considered non-standard). In TF-A, setting the + ``W`` build flag to ``W=3`` enables the *-Wpointer-arith* compiler flag and + this will emit warnings where pointer arithmetic is used. - struct my_struct { - int arg1; - int arg2; - }; + - Use ``ptrdiff_t`` to compare the difference between 2 pointers. - void init(struct my_struct *ptr); +- Use ``size_t`` when storing the ``sizeof()`` something. - void main(void) - { - struct my_struct x; - x.arg1 = 1; - x.arg2 = 2; - init(&x); - } +- Use ``ssize_t`` when returning the ``sizeof()`` something from a function that + can also return an error code; the signed type allows for a negative return + code in case of error. This practice should be used sparingly. -is better written as: +- Use ``u_register_t`` when it's important to store the contents of a register + in its native size (32-bit in |AArch32| and 64-bit in |AArch64|). This is not a + standard *C99* type but is widely available in libc implementations, + including the FreeBSD version included with the TF codebase. Where possible, + cast the variable to a more appropriate type before interpreting the data. For + example, the following struct in ``ep_info.h`` could use this type to minimize + the storage required for the set of registers: .. code:: c - struct my_struct { - int arg1; - int arg2; - }; - - void init(const struct my_struct *ptr); - - void main(void) - { - const struct my_struct x = { 1, 2 }; - init(&x); - } - -This allows the linker to put the data in a read-only data section instead of a -writeable data section, which may result in a smaller and faster binary. Note -that this may require dependent functions (``init()`` in the above example) to -have ``const`` arguments, assuming they don't need to modify the data. - -Library and driver code ------------------------ - -TF library code (under ``lib/`` and ``include/lib``) is any code that provides a -reusable interface to other code, potentially even to code outside of TF. - -In some systems drivers must conform to a specific driver framework to provide -services to the rest of the system. TF has no driver framework and the -distinction between a driver and library is somewhat subjective. + typedef struct aapcs64_params { + u_register_t arg0; + u_register_t arg1; + u_register_t arg2; + u_register_t arg3; + u_register_t arg4; + u_register_t arg5; + u_register_t arg6; + u_register_t arg7; + } aapcs64_params_t; -A driver (under ``drivers/`` and ``include/drivers/``) is defined as code that -interfaces with hardware via a memory mapped interface. +If some code wants to operate on ``arg0`` and knows that it represents a 32-bit +unsigned integer on all systems, cast it to ``unsigned int``. -Some drivers (for example, the Arm CCI driver in ``include/drivers/arm/cci.h``) -provide a general purpose API to that specific hardware. Other drivers (for -example, the Arm PL011 console driver in ``drivers/arm/pl011/pl011_console.S``) -provide a specific hardware implementation of a more abstract library API. In -the latter case there may potentially be multiple drivers for the same hardware -device. +These guidelines should be updated if additional types are needed. -Neither libraries nor drivers should depend on platform-specific code. If they -require platform-specific data (for example, a base address) to operate then -they should provide an initialization function that takes the platform-specific -data as arguments. +-------------- -TF common code (under ``common/`` and ``include/common/``) is code that is re-used -by other generic (non-platform-specific) TF code. It is effectively internal -library code. +*Copyright (c) 2020, Arm Limited and Contributors. All rights reserved.* +.. _`Linux master tree`: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/ +.. _`Procedure Call Standard for the Arm Architecture`: https://developer.arm.com/docs/ihi0042/latest/ +.. _`Procedure Call Standard for the Arm 64-bit Architecture`: https://developer.arm.com/docs/ihi0055/latest/ +.. _`EditorConfig`: http://editorconfig.org/ .. _`Why the “volatile” type class should not be used`: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html -.. _`Procedure Call Standard for the Arm Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf -.. _`Procedure Call Standard for the Arm 64-bit Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf +.. _`MISRA C:2012 Guidelines`: https://www.misra.org.uk/Activities/MISRAC/tabid/160/Default.aspx +.. _`a spreadsheet`: https://developer.trustedfirmware.org/file/download/lamajxif3w7c4mpjeoo5/PHID-FILE-fp7c7acszn6vliqomyhn/MISRA-and-TF-Analysis-v1.3.ods -- cgit v1.2.3 From 9061c0c9ab70bea29a56f1ea73f8f0f8859f0bab Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux Date: Thu, 20 Aug 2020 10:41:36 +0200 Subject: doc: Minor formatting improvement in the coding guidelines document Change-Id: I5362780db422772fd547dc8e68e459109edccdd0 Signed-off-by: Sandrine Bailleux --- docs/process/coding-guidelines.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'docs/process/coding-guidelines.rst') diff --git a/docs/process/coding-guidelines.rst b/docs/process/coding-guidelines.rst index f7d53a97e..97086047d 100644 --- a/docs/process/coding-guidelines.rst +++ b/docs/process/coding-guidelines.rst @@ -95,10 +95,13 @@ By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will be compiled into debug builds and all statements with a log level ``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be overridden from the command line or by the platform makefile (although it may be -necessary to clean the build directory first). For example, to enable -``VERBOSE`` logging on FVP: +necessary to clean the build directory first). -``make PLAT=fvp LOG_LEVEL=50 all`` +For example, to enable ``VERBOSE`` logging on FVP: + +.. code:: shell + + make PLAT=fvp LOG_LEVEL=50 all Use const data where possible ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- cgit v1.2.3 From 06ffa16694b315380c9b2ebafe06d83873f2a78d Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux Date: Thu, 20 Aug 2020 11:17:41 +0200 Subject: doc: Recommend using C rather than assembly language Add a section for that in the coding guidelines. Change-Id: Ie6819c4df5889a861460eb96acf2bc9c0cfb494e Signed-off-by: Sandrine Bailleux --- docs/process/coding-guidelines.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'docs/process/coding-guidelines.rst') diff --git a/docs/process/coding-guidelines.rst b/docs/process/coding-guidelines.rst index 97086047d..a8a84549a 100644 --- a/docs/process/coding-guidelines.rst +++ b/docs/process/coding-guidelines.rst @@ -441,6 +441,25 @@ unsigned integer on all systems, cast it to ``unsigned int``. These guidelines should be updated if additional types are needed. +Favor C language over assembly language +--------------------------------------- + +Generally, prefer code written in C over assembly. Assembly code is less +portable, harder to understand, maintain and audit security wise. Also, static +analysis tools generally don't analyze assembly code. + +There are, however, legitimate uses of assembly language. These include: + + - Early boot code executed before the C runtime environment is setup. + + - Exception handling code. + + - Low-level code where the exact sequence of instructions executed on the CPU + matters, such as CPU reset sequences. + + - Low-level code where specific system-level instructions must be used, such + as cache maintenance operations. + -------------- *Copyright (c) 2020, Arm Limited and Contributors. All rights reserved.* -- cgit v1.2.3 From 7969747e7f7819b6c66977d85b776c0a9a169c1b Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux Date: Fri, 14 Aug 2020 15:58:50 +0200 Subject: doc: Improve contribution guidelines - Add some guidance about the type of information a patch author should provide to facilitate the review (and for future reference). - Make a number of implicit expectations explicit: - Every patch must compile. - All CI tests must pass. - Mention that the patch author is expected to add reviewers and explain how to choose them. - Explain the patch submission rules in terms of Gerrit labels. Also do some cosmetic changes, like adding empty lines, shuffling some paragraphs around. Change-Id: I6dac486684310b5a35aac7353e10fe5474a81ec5 Signed-off-by: Sandrine Bailleux --- docs/process/coding-guidelines.rst | 1 + 1 file changed, 1 insertion(+) (limited to 'docs/process/coding-guidelines.rst') diff --git a/docs/process/coding-guidelines.rst b/docs/process/coding-guidelines.rst index 97086047d..2c8620d15 100644 --- a/docs/process/coding-guidelines.rst +++ b/docs/process/coding-guidelines.rst @@ -19,6 +19,7 @@ support its functionality through plugins. Use of the EditorConfig file is suggested but is not required. +.. _automatic-compliance-checking: Automatic Compliance Checking ----------------------------- -- cgit v1.2.3