diff options
Diffstat (limited to 'Documentation/coding-style.rst')
| -rw-r--r-- | Documentation/coding-style.rst | 141 |
1 files changed, 100 insertions, 41 deletions
diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index bbc1f2fb..6ac3a4a0 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -1,3 +1,7 @@ +.. SPDX-License-Identifier: CC-BY-SA-4.0 + +.. include:: documentation-contents.rst + .. _coding-style-guidelines: Coding Style Guidelines @@ -57,7 +61,7 @@ document: underscores in between * All formatting rules specified in the selected sections of the Linux kernel Code Style for indentation, braces, spacing, etc -* Header guards are formatted as '__LIBCAMERA_FILE_NAME_H__' +* Headers are guarded by the use of '#pragma once' Order of Includes ~~~~~~~~~~~~~~~~~ @@ -68,31 +72,41 @@ macro. For .cpp files, if the file implements an API declared in a header file, that header file shall be included first in order to ensure it is self-contained. -The headers shall be grouped and ordered as follows. +While the following list is extensive, it documents the expected behaviour +defined by the clang-format configuration and tooling should assist with +ordering. + +The headers shall be grouped and ordered as follows: - # The header declaring the API being implemented (if any) - # The C and C++ system and standard library headers - # Other libraries' headers, with one group per library - # Other project's headers +1. The header declaring the API being implemented (if any) +2. The C and C++ system and standard library headers +3. Linux kernel headers +4. The libcamera base private header if required +5. The libcamera base library headers +6. The libcamera public API headers +7. The libcamera IPA interfaces +8. The internal libcamera headers +9. Other libraries' headers, with one group per library +10. Local headers grouped by subdirectory +11. Any local headers Groups of headers shall be separated by a single blank line. Headers within each group shall be sorted alphabetically. System and library headers shall be included with angle brackets. Project headers shall be included with angle brackets for the libcamera public API -headers, and with double quotes for other libcamera headers. +headers, and with double quotes for internal libcamera headers. C++ Specific Rules ------------------ -The code shall be implemented in C++14, with the following caveats: +The code shall be implemented in C++17, with the following caveats: * Type inference (auto and decltype) shall be used with caution, to avoid drifting towards an untyped language. * The explicit, override and final specifiers are to be used where applicable. -* General-purpose smart pointers (std::unique_ptr) deprecate std::auto_ptr. - Smart pointers, as well as shared pointers and weak pointers, shall not be +* Smart pointers, as well as shared pointers and weak pointers, shall not be overused. * Classes are encouraged to define move constructors and assignment operators where applicable, and generally make use of the features offered by rvalue @@ -116,7 +130,7 @@ reference means using a reference passed by a caller without ownership transfer based on the assumption that the caller guarantees the validity of the reference for the duration of the operation that borrows it. -#. Single Owner Objects +1. Single Owner Objects * By default an object has a single owner at any time. * Storage of single owner objects varies depending on how the object @@ -153,7 +167,7 @@ reference for the duration of the operation that borrows it. otherwise specified, pointers passed to functions are considered as borrowed references valid for the duration of the function only. -#. Shared Objects +2. Shared Objects * Objects that may have multiple owners at a given time are called shared objects. They are reference-counted and live as long as any references to @@ -186,6 +200,55 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide` long term borrowing isn't marked through language constructs, it shall be documented explicitly in details in the API. +Global Variables +~~~~~~~~~~~~~~~~ + +The order of initializations and destructions of global variables cannot be +reasonably controlled. This can cause problems (including segfaults) when global +variables depend on each other, directly or indirectly. For example, if the +declaration of a global variable calls a constructor which uses another global +variable that hasn't been initialized yet, incorrect behavior is likely. +Similar issues may occur when the library is unloaded and global variables are +destroyed. + +Global variables that are statically initialized and have trivial destructors +(such as an integer constant) do not cause any issue. Other global variables +shall be avoided when possible, but are allowed when required (for instance to +implement factories with auto-registration). They shall not depend on any other +global variable, should run a minimal amount of code in the constructor and +destructor, and code that contains dependencies should be moved to a later +point in time. + +Error Handling +~~~~~~~~~~~~~~ + +Proper error handling is crucial to the stability of libcamera. The project +follows a set of high-level rules: + +* Make errors impossible through API design. The best way to handle errors is + to prevent them from happening in the first place. The preferred option is + thus to prevent error conditions at the API design stage when possible. +* Detect errors at compile time. Compile-test checking of errors not only + reduces the runtime complexity, but also ensures that errors are caught early + on during development instead of during testing or, worse, in production. The + static_assert() declaration should be used where possible for this purpose. +* Validate all external API contracts. Explicit pre-condition checks shall be + used to validate API contracts. Whenever possible, appropriate errors should + be returned directly. As libcamera doesn't use exceptions, errors detected in + constructors shall result in the constructed object being marked as invalid, + with a public member function available to check validity. The checks should + be thorough for the public API, and may be lighter for internal APIs when + pre-conditions can reasonably be considered to be met through other means. +* Use assertions for fatal issues only. The ASSERT() macro causes a program + abort when compiled in debug mode, and is a no-op otherwise. It is useful to + abort execution synchronously with the error check instead of letting the + error cause problems (such as segmentation faults) later, and to provide a + detailed backtrace. Assertions shall only be used to catch conditions that are + never supposed to happen without a serious bug in libcamera that would prevent + safe recovery. They shall never be used to validate API contracts. The + assertion conditions shall not cause any side effect as they are compiled out + in non-debug mode. + C Compatibility Headers ~~~~~~~~~~~~~~~~~~~~~~~ @@ -194,9 +257,17 @@ them, defines C compatibility headers. The former have a name of the form <cxxx> while the later are named <xxx.h>. The C++ headers declare names in the std namespace, and may declare the same names in the global namespace. The C compatibility headers declare names in the global namespace, and may declare -the same names in the std namespace. Usage of the C compatibility headers is -strongly preferred. Code shall not rely on the optional declaration of names in -the global or std namespace. +the same names in the std namespace. Code shall not rely on the optional +declaration of names in the global or std namespace. + +Usage of the C compatibility headers is preferred, except for the math.h header. +Where math.h defines separate functions for different argument types (e.g. +abs(int), labs(long int), fabs(double) and fabsf(float)) and requires the +developer to pick the right function, cmath defines overloaded functions +(std::abs(int), std::abs(long int), std::abs(double) and std::abs(float) to let +the compiler select the right function. This avoids potential errors such as +calling abs(int) with a float argument, performing an unwanted implicit integer +conversion. For this reason, cmath is preferred over math.h. Documentation @@ -223,7 +294,7 @@ member function of the PipelineHandler class. * \param[in] camera The camera to start * * Start the group of streams that have been configured for capture by - * \a configureStreams(). The intended caller of this method is the Camera + * \a configureStreams(). The intended caller of this function is the Camera * class which will in turn be called from the application to indicate that * it has configured the streams and is ready to capture. * @@ -255,28 +326,12 @@ The 'clang-format' code formatting tool can be used to reformat source files with the libcamera coding style, defined in the .clang-format file at the root of the source tree. -Alternatively the 'astyle' tool can also be used, with the following arguments. - -:: - - --style=linux - --indent=force-tab=8 - --attach-namespaces - --attach-extern-c - --pad-oper - --align-pointer=name - --align-reference=name - --max-code-length=120 - -Use of astyle is discouraged as clang-format better matches the libcamera coding -style. - -As both astyle and clang-format are code formatters, they operate on full files -and output reformatted source code. While they can be used to reformat code -before sending patches, it may generate unrelated changes. To avoid this, -libcamera provides a 'checkstyle.py' script wrapping the formatting tools to -only retain related changes. This should be used to validate modifications -before submitting them for review. +As clang-format is a code formatter, it operates on full files and outputs +reformatted source code. While it can be used to reformat code before sending +patches, it may generate unrelated changes. To avoid this, libcamera provides a +'checkstyle.py' script wrapping the formatting tools to only retain related +changes. This should be used to validate modifications before submitting them +for review. The script operates on one or multiple git commits specified on the command line. It does not modify the git tree, the index or the working directory and @@ -365,8 +420,12 @@ diff that fixes the issues, on top of the corresponding commit. As the script is in early development false positive are expected. The flagged issues should be reviewed, but the diff doesn't need to be applied blindly. -The checkstyle.py script uses clang-format by default if found, and otherwise -falls back to astyle. The formatter can be manually selected with the -'--formatter' argument. +Execution of checkstyle.py can be automated through git commit hooks. Example +of pre-commit and post-commit hooks are available in `utils/hooks/pre-commit` +and `utils/hooks/post-commit`. You can install either hook by copying it to +`.git/hooks/`. The post-commit hook is easier to start with as it will only flag +potential issues after committing, while the pre-commit hook will abort the +commit if issues are detected and requires usage of `git commit --no-verify` to +ignore false positives. Happy hacking, libcamera awaits your patches! |