Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 1 | # Style guide |
| 2 | |
| 3 | Hafnium's coding style has been based on the |
| 4 | [Linux style](https://www.kernel.org/doc/html/v4.17/process/coding-style.html) |
| 5 | with explicit modifications: |
| 6 | |
| 7 | * Always use braces for conditionals and loops. (No SSL `goto fail;`, thanks.) |
| 8 | |
| 9 | Following this, we generally fall back to the subset of the |
| 10 | [Google C++ style guide](https://google.github.io/styleguide/cppguide.html) that |
| 11 | is applicable to C. |
| 12 | |
| 13 | We try to automate this where possible with clang-format and clang-tidy but that |
| 14 | doesn't capture everything we'd like today. Where the style enforced by this |
| 15 | tooling conflicts with what is in this document we accept what the tooling |
| 16 | requires, and try to improve it if possible. |
| 17 | |
Andrew Walbran | b784997 | 2019-11-15 15:23:43 +0000 | [diff] [blame] | 18 | [TOC] |
| 19 | |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 20 | ## Clarifications |
| 21 | |
| 22 | * Yes, it does mean all variables are declared, C90-style, at the top of |
| 23 | scope, even those loop induction variables. |
| 24 | * Linux encourages no braces around single-statement branches. We follow |
| 25 | Google and require braces around all scope blocks. |
| 26 | |
| 27 | ## Naming symbols |
| 28 | |
| 29 | * Arch-specific functions should start with `arch_`. |
| 30 | * Platform-specific functions should start with `plat_`. |
Shaked Flur | b06f12f | 2019-12-11 14:49:14 +0000 | [diff] [blame] | 31 | * Non-static functions should generally start with the name of the file they |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 32 | are declared in (after the `arch_` or `plat_` prefix if appropriate), though |
| 33 | there are quite a few exceptions to this rule. |
| 34 | * Prefer `x_count` over `num_x`. |
| 35 | |
| 36 | ## Prose |
| 37 | |
| 38 | These rules apply to comments and other natural language text. |
| 39 | |
| 40 | * Capitalize acronyms. |
| 41 | * CPU, vCPU, VM, EL2, SPCI, QEMU |
| 42 | * Spell out Hafnium in full, not Hf. |
| 43 | * Use single spaces. |
| 44 | * Sentences end with full stops. |
| 45 | * If the comment fits on one line use `/* */`, otherwise space it out: |
| 46 | |
| 47 | ``` |
| 48 | /* |
| 49 | * Informative long comment |
| 50 | * with extra information. |
| 51 | */ |
| 52 | ``` |
| 53 | |
| 54 | * Doc-ish comments start with `/**`. |
| 55 | |
| 56 | * Use for: |
| 57 | * Function definitions (not declarations) |
| 58 | * Struct declarations |
| 59 | * Enum values |
| 60 | * Do not use for: |
| 61 | * Macros |
| 62 | * Definitions of globals |
| 63 | |
| 64 | * References to code symbols use backticks, e.g. `` `my_symbol` ``. |
| 65 | |
| 66 | ## Coding practices |
| 67 | |
| 68 | * Function macros should be functions instead, that way you get types. |
Andrew Walbran | 5c38fb7 | 2020-02-10 15:00:20 +0000 | [diff] [blame] | 69 | * Lock ordering is described at the top of [`api.c`](../src/api.c). |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 70 | * Use opaque types to avoid implicit casts when it will help avoid mistakes. |
Andrew Walbran | 5c38fb7 | 2020-02-10 15:00:20 +0000 | [diff] [blame] | 71 | e.g. [`addr.h`](../inc/hf/addr.h) |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 72 | * Avoid inline casting. C doesn't give much protection so be formal about the |
Andrew Walbran | 5c38fb7 | 2020-02-10 15:00:20 +0000 | [diff] [blame] | 73 | transformations. e.g. [`addr.h`](../inc/hf/addr.h) |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 74 | * If a function acquires a resource, there must be a single exit path to free |
| 75 | the resource. Tracking down multiple exit points is hard and requires |
| 76 | duplicated code which is harder. This may require splitting functions into |
| 77 | subfunctions. Early exit is okay if there aren't any clean up tasks. |
| 78 | * Don't use function pointers. It makes analysis hard and is often a target of |
| 79 | attacks. |
Andrew Walbran | 5c38fb7 | 2020-02-10 15:00:20 +0000 | [diff] [blame] | 80 | * Be liberal with `CHECK`. Use it to assert pre-/post- conditions. |
Andrew Walbran | d0d0e34 | 2019-10-17 14:11:42 +0100 | [diff] [blame] | 81 | * No self-modifying code. |
| 82 | * Build targets should include all the direct dependencies for their sources, |
| 83 | where possible, rather than relying on transitive dependencies. |
Andrew Walbran | 17eebf9 | 2020-02-05 16:35:49 +0000 | [diff] [blame] | 84 | |
| 85 | ## Logging |
| 86 | |
| 87 | Hafnium uses the same log levels as Arm Trusted Firmware. There are 5 log |
| 88 | levels, in order of severity: |
| 89 | |
| 90 | 1. `ERROR` |
| 91 | |
| 92 | Use this only for cases that there is an error in the hypervisor itself, |
| 93 | perhaps caused by a coding error, bad configuration, unexpected hardware |
| 94 | behaviour or a malformed manifest. Errors should not be logged during normal |
| 95 | operation, even in case of a buggy or malicious VM. |
| 96 | |
| 97 | 2. `NOTICE` |
| 98 | |
| 99 | Use this sparingly for important messages which should be logged even in |
| 100 | production builds because they will be useful for debugging. This is a |
| 101 | suitable level to use for events which may indicate a bug in a VM. |
| 102 | |
| 103 | 3. `WARNING` |
| 104 | |
| 105 | Use this for warnings which are important to developers but can generally be |
| 106 | ignored in production. |
| 107 | |
| 108 | 4. `INFO` |
| 109 | |
| 110 | Use this to provide extra information that is helpful for developers. |
| 111 | |
| 112 | 5. `VERBOSE` |
| 113 | |
| 114 | Use this to provide even more information which may be helpful when tracing |
| 115 | through execution in detail, such as when debugging test failures. This is |
| 116 | the only level which should include any sensitive data. |
| 117 | |
| 118 | Logging is done with the `dlog_*` macros, e.g. `dlog_info`. These accept |
| 119 | printf-style format strings and arguments. |
| 120 | |
| 121 | The log level of a build is controlled by the `log_level` argument defined in |
| 122 | [`BUILDCONFIG.gn`](../build/BUILDCONFIG.gn). This defaults to `INFO` for debug |
| 123 | builds and tests, meaning that all levels except `VERBOSE` will be logged. It is |
| 124 | recommended to set the log level to `NOTICE` for production builds, to reduce |
| 125 | binary size and log spam. |