doc/style_guide.md

   1 # Bluedroid Style Guide
   2 This document outlines the coding conventions and code style used in Bluedroid.
   3 Its primary purpose is to provide explicit guidance on style so that developers
   4 are consistent with one another and spend less time debating style.
   5
   6 ## Directory structure
   7 Directories at the top-level should consist of major subsystems in Bluedroid.
   8 Each subsystem's purpose should be documented in the `doc/directory_layout.md`
   9 file, even if it seems obvious from the name.
  10
  11 For a subsystem that contains code, its directory structure should look like:
  12 ```
  13   Android.mk
  14   include/
  15   src/
  16   test/
  17 ```
  18 Further, the directory structure inside `src/` and `include/` should be
  19 mirrored. In other words, if `src/` contains a subdirectory called `foo/`,
  20 `include/` must also have a subdirectory named `foo/`.
  21
  22 ## Target architecture
  23 Bluedroid targets a variety of hardware and cannot make many assumptions about
  24 memory layout, sizes, byte order, etc. As a result, some operations are
  25 considered unsafe and this section outlines the most important ones to watch out
  26 for.
  27
  28 ### Pointer / integer casts
  29 In general, do not cast pointers to integers or vice versa.
  30
  31 The one exception is if an integer needs to be temporarily converted to a
  32 pointer and then back to the original integer. This style of code is typically
  33 needed when providing an integral value as the context to a callback, as in the
  34 following example.
  35 ```
  36 void my_callback(void *context) {
  37   uintptr_t arg = context;
  38 }
  39
  40 set_up_callback(my_callback, (uintptr_t)5);
  41 ```
  42 Note, however, that the integral value was written into the pointer and read
  43 from the pointer as a `uintptr_t` to avoid a loss of precision (or to make the
  44 loss explicit).
  45
  46 ### Byte order
  47 It is not safe to assume any particular byte order. When serializing or
  48 deserializing data, it is unsafe to memcpy unless both source and destination
  49 pointers have the same type.
  50
  51 ## Language
  52 Bluedroid is written in C99 and should take advantage of the features offered by
  53 it. However, not all language features lend themselves well to the type of
  54 development required by Bluedroid. This section provides guidance on some of the
  55 features to embrace or avoid.
  56
  57 ### C Preprocessor
  58 The use of the C preprocessor should be minimized. In particular:
  59 * use functions or, if absolutely necessary, inline functions instead of macros
  60 * use `static const` variables instead of `#define`
  61 * use `enum` for enumerations, not a collection of `#define`s
  62 * minimize the use of feature / conditional macros
  63
  64 The last point is perhaps the most contentious. It's well-understood that
  65 feature macros are useful in reducing code size but it leads to an exponential
  66 explosion in build configurations. Setting up, testing, and verifying each of
  67 the `2^n` build configurations is untenable for `n` greater than, say, 4.
  68
  69 ### C++
  70 Although C++ offers constructs that may make Bluedroid development faster,
  71 safer, more pleasant, etc. the decision _for the time being_ is to stick with
  72 pure C99. The exceptions are when linking against libraries that are written
  73 in C++. At the time of writing these libraries are `gtest` and `tinyxml2`,
  74 where the latter is a dependency that should be eliminated in favor of simpler,
  75 non-XML formats.
  76
  77 ### Variadic functions
  78 Variadic functions are dangerous and should be avoided for most code. The
  79 exception is when implementing logging since the benefits of readability
  80 outweigh the cost of safety.
  81
  82 ### Functions with zero arguments
  83 Functions that do not take any arguments (0 arity) should be declared like so:
  84 ```
  85 void function(void);
  86 ```
  87 Note that the function explicitly includes `void` in its parameter list to
  88 indicate to the compiler that it takes no arguments.
  89
  90 ### Variable declarations
  91 Variables should be declared one per line as close to initialization as possible.
  92 In nearly all cases, variables should be declared and initialized on the same line.
  93 Variable declarations should not include extra whitespace to line up fields. For
  94 example, the following style is preferred:
  95 ```
  96   int my_long_variable_name = 0;
  97   int x = 5;
  98 ```
  99 whereas this code is not acceptable:
 100 ```
 101   int my_long_variable_name = 0;
 102   int                     x = 5;
 103 ```
 104
 105 As a result of the above rule to declare and initialize variables together,
 106 `for` loops should declare and initialize their iterator variable in the
 107 initializer statement:
 108 ```
 109   for (int i = 0; i < 10; ++i) {
 110     // use i
 111   }
 112 ```
 113
 114 ### Contiguous memory structs
 115 Use C99 flexible arrays as the last member of a struct if the array needs
 116 to be allocated in contiguous memory with its containing struct.
 117 A flexible array member is writen as `array_name[]` without a specified size.
 118 For example:
 119 ```
 120 typedef struct {
 121   size_t length;
 122   uint8_t data[];
 123 } buffer_t;
 124
 125 // Allocate a buffer with 128 bytes available for my_buffer->data.
 126 buffer_t *my_buffer = malloc(sizeof(buffer_t) + 128);
 127 uint8_t *data = my_buffer->data;
 128 ```
 129
 130 ### Pointer arithmetic
 131 Avoid pointer arithmetic when possible as it results in difficult to read code.
 132 Prefer array-indexing syntax over pointer arithmetic.
 133
 134 In particular, do not write code like this:
 135 ```
 136 typedef struct {
 137   size_t length;
 138 } buffer_t;
 139
 140 buffer_t *my_buffer = malloc(sizeof(buffer_t) + 128);
 141 uint8_t *data = (uint8_t *)(my_buffer + 1);
 142 ```
 143 Instead, use zero-length arrays as described above to avoid pointer arithmetic
 144 and array indexing entirely.
 145
 146 ### Boolean type
 147 Use the C99 `bool` type with values `true` and `false` defined in `stdbool.h`.
 148 Not only is this a standardized type, it is also safer and provides more
 149 compile-time checks.
 150
 151 ### Booleans instead of bitfields
 152 Use booleans to represent boolean state, instead of a set of masks into an
 153 integer. It's more transparent and readable, and less error prone.
 154
 155 ## Header files
 156 In general, every source file (`.c` or `.cpp`) in a `src/` directory should
 157 have a corresponding header (`.h`) in the `include/` directory.
 158
 159 ### Template header file
 160 ```
 161 [copyright header]
 162
 163 #pragma once
 164
 165 #include <system/a.h>
 166 #include <system/b.h>
 167
 168 #include "subsystem/include/a.h"
 169 #include "subsystem/include/b.h"
 170
 171 typedef struct alarm_t alarm_t;
 172 typedef struct list_t list_t;
 173
 174 // This comment describes the following function. It is not a structured
 175 // comment, it's English prose. Function arguments can be referred to as
 176 // |param|. This function returns true if a new object was created, false
 177 // otherwise.
 178 bool template_new(const list_t *param);
 179
 180 // Each public function must have a comment describing its semantics. In
 181 // particular, edge cases, and whether a pointer argument may or may not be
 182 // NULL.
 183 void template_use_alarm(alarm_t *alarm);
 184 ```
 185
 186 ### License header
 187 Each header file must begin with the following Apache 2.0 License with `<year>`
 188 and `<owner>` replaced with the year in which the file was authored and the
 189 owner of the copyright, respectively.
 190 ```
 191 /******************************************************************************
 192  *
 193  *  Copyright (C) <year> <owner>
 194  *
 195  *  Licensed under the Apache License, Version 2.0 (the "License");
 196  *  you may not use this file except in compliance with the License.
 197  *  You may obtain a copy of the License at:
 198  *
 199  *  http://www.apache.org/licenses/LICENSE-2.0
 200  *
 201  *  Unless required by applicable law or agreed to in writing, software
 202  *  distributed under the License is distributed on an "AS IS" BASIS,
 203  *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 204  *  See the License for the specific language governing permissions and
 205  *  limitations under the License.
 206  *
 207  ******************************************************************************/
 208 ```
 209
 210 ### Include guard
 211 After the license header, each header file must contain the include guard:
 212 ```
 213 #pragma once
 214 ```
 215 This form is used over traditional `#define`-based include guards as it is less
 216 error-prone, doesn't pollute the global namespace, is more compact, and can
 217 result in faster compilation.
 218
 219 ## Formatting
 220 Code formatting is pretty arbitrary, but the codebase is easier to follow if
 221 everyone uses the same style. Individuals may not agree with every aspect of
 222 the formatting rules, and some of the rules may take some getting used to,
 223 but it is important that all engineers follow the formatting rules so we can all
 224 understand and read the code easily.
 225
 226 ### White space
 227 * use only spaces, indent 2 spaces at a time
 228 * no trailing whitespaces at the end of a line
 229 * no tab characters
 230 * use one blank line to separate logical code blocks, function definitions,
 231   and sections of a file
 232
 233 ```
 234 // Space after keyword in conditionals and loops.
 235 // No space immeidately before or after parentheses.
 236 if (foo)
 237
 238 // Space surrounding binary operators.
 239 if (foo < 5)
 240
 241 // Space after comma.
 242 for (int x = 0, y = 0; x; ++y)
 243
 244 // No space between unary operators and their argument.
 245 ++x;
 246 z = -y;
 247
 248 // No space between function name and open parenthesis.
 249 call_my_fn(arg1, arg2);
 250
 251 // Space before * in variable declaration.
 252 int *x = NULL;
 253
 254 // Space after // beginning a comment.
 255 // Notice the space between "//" and "N".
 256 ```
 257
 258 Use only spaces, and indent 2 spaces at a time. Do not use tab characters in the
 259 codebase.
 260
 261 Use a single blank line to separate logical code blocks, function definitions,
 262 and sections of a file.
 263
 264 ### Brace style
 265 ```
 266 // Open curly braces are never on a line by themselves.
 267 void my_function(void) {
 268   // Conditional statements with only one child statement should elide braces.
 269   // The child statement must be on a new line, indented by 2 spaces.
 270   if (foo)
 271     do_bar();
 272   else
 273     do_baz();
 274
 275   // Conditionals with a branch containing more than one child statement forces
 276   // braces on both branches.
 277   if (foo) {
 278     do_bar();
 279   } else {
 280     do_baz();
 281     ++var1;
 282   }
 283 }
 284 ```