1 # Fluoride Style Guide 2 This document outlines the coding conventions and code style used in Fluoride. 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 As a rule, we follow the Google C++ 7 [Style Guide](https://google.github.io/styleguide/cppguide.html). 8 Exceptions will be noted below. 9 10 ## Directory structure 11 Directories at the top-level should consist of major subsystems in Fluoride. 12 Each subsystem's purpose should be documented in the `doc/directory_layout.md` 13 file, even if it seems obvious from the name. 14 15 For a subsystem that contains code, its directory structure should look like: 16 ``` 17 Android.mk 18 include/ 19 src/ 20 test/ 21 ``` 22 Further, the directory structure inside `src/` and `include/` should be 23 mirrored. In other words, if `src/` contains a subdirectory called `foo/`, 24 `include/` must also have a subdirectory named `foo/`. 25 26 ## Target architecture 27 Fluoride targets a variety of hardware and cannot make many assumptions about 28 memory layout, sizes, byte order, etc. As a result, some operations are 29 considered unsafe and this section outlines the most important ones to watch out 30 for. 31 32 ### Pointer / integer casts 33 In general, do not cast pointers to integers or vice versa. 34 35 The one exception is if an integer needs to be temporarily converted to a 36 pointer and then back to the original integer. This style of code is typically 37 needed when providing an integral value as the context to a callback, as in the 38 following example. 39 ``` 40 void my_callback(void *context) { 41 uintptr_t arg = context; 42 } 43 44 set_up_callback(my_callback, (uintptr_t)5); 45 ``` 46 Note, however, that the integral value was written into the pointer and read 47 from the pointer as a `uintptr_t` to avoid a loss of precision (or to make the 48 loss explicit). 49 50 ### Byte order 51 It is not safe to assume any particular byte order. When serializing or 52 deserializing data, it is unsafe to memcpy unless both source and destination 53 pointers have the same type. 54 55 ## Language 56 Fluoride is written in C99 and should take advantage of the features offered by 57 it. However, not all language features lend themselves well to the type of 58 development required by Fluoride. This section provides guidance on some of the 59 features to embrace or avoid. 60 61 ### C Preprocessor 62 The use of the C preprocessor should be minimized. In particular: 63 * use functions or, if absolutely necessary, inline functions instead of macros 64 * use `static const` variables instead of `#define` 65 * use `enum` for enumerations, not a collection of `#define`s 66 * minimize the use of feature / conditional macros 67 68 The last point is perhaps the most contentious. It's well-understood that 69 feature macros are useful in reducing code size but it leads to an exponential 70 explosion in build configurations. Setting up, testing, and verifying each of 71 the `2^n` build configurations is untenable for `n` greater than, say, 4. 72 73 ### C++ 74 Although C++ offers constructs that may make Fluoride development faster, 75 safer, more pleasant, etc. the decision _for the time being_ is to stick with 76 pure C99. The exceptions are when linking against libraries that are written 77 in C++. At the time of writing these libraries are `gtest` and `tinyxml2`, 78 where the latter is a dependency that should be eliminated in favor of simpler, 79 non-XML formats. 80 81 ### Variadic functions 82 Variadic functions are dangerous and should be avoided for most code. The 83 exception is when implementing logging since the benefits of readability 84 outweigh the cost of safety. 85 86 ### Functions with zero arguments 87 Functions that do not take any arguments (0 arity) should be declared like so: 88 ``` 89 void function(void); 90 ``` 91 Note that the function explicitly includes `void` in its parameter list to 92 indicate to the compiler that it takes no arguments. 93 94 ### Variable declarations 95 Variables should be declared one per line as close to initialization as possible. 96 In nearly all cases, variables should be declared and initialized on the same line. 97 Variable declarations should not include extra whitespace to line up fields. For 98 example, the following style is preferred: 99 ``` 100 int my_long_variable_name = 0; 101 int x = 5; 102 ``` 103 whereas this code is not acceptable: 104 ``` 105 int my_long_variable_name = 0; 106 int x = 5; 107 ``` 108 109 As a result of the above rule to declare and initialize variables together, 110 `for` loops should declare and initialize their iterator variable in the 111 initializer statement: 112 ``` 113 for (int i = 0; i < 10; ++i) { 114 // use i 115 } 116 ``` 117 118 ### Contiguous memory structs 119 Use C99 flexible arrays as the last member of a struct if the array needs 120 to be allocated in contiguous memory with its containing struct. 121 A flexible array member is writen as `array_name[]` without a specified size. 122 For example: 123 ``` 124 typedef struct { 125 size_t length; 126 uint8_t data[]; 127 } buffer_t; 128 129 // Allocate a buffer with 128 bytes available for my_buffer->data. 130 buffer_t *my_buffer = malloc(sizeof(buffer_t) + 128); 131 uint8_t *data = my_buffer->data; 132 ``` 133 134 ### Pointer arithmetic 135 Avoid pointer arithmetic when possible as it results in difficult to read code. 136 Prefer array-indexing syntax over pointer arithmetic. 137 138 In particular, do not write code like this: 139 ``` 140 typedef struct { 141 size_t length; 142 } buffer_t; 143 144 buffer_t *my_buffer = malloc(sizeof(buffer_t) + 128); 145 uint8_t *data = (uint8_t *)(my_buffer + 1); 146 ``` 147 Instead, use zero-length arrays as described above to avoid pointer arithmetic 148 and array indexing entirely. 149 150 ### Boolean type 151 Use the C99 `bool` type with values `true` and `false` defined in `stdbool.h`. 152 Not only is this a standardized type, it is also safer and provides more 153 compile-time checks. 154 155 ### Booleans instead of bitfields 156 Use booleans to represent boolean state, instead of a set of masks into an 157 integer. It's more transparent and readable, and less error prone. 158 159 ### Function names as strings 160 C99 defines `__func__` as an identifier that represents the function's name 161 in which it is used. The magic identifier `__FUNCTION__` should not be used 162 as it is a non-standard language extension and an equivalent standardized 163 mechanism exists. In other words, use `__func__` over `__FUNCTION__`. 164 165 ## Fluoride conventions 166 This section describes coding conventions that are specific to Fluoride. 167 Whereas the _Language_ section describes the use of language features, this 168 section describes idioms, best practices, and conventions that are independent 169 of language features. 170 171 ### Memory management 172 Use `osi_malloc` or `osi_calloc` to allocate bytes instead of plain `malloc`. 173 Likewise, use `osi_free` over `free`. These wrapped functions provide additional 174 lightweight memory bounds checks that can help track down memory errors. 175 176 By convention, functions that contain `*_new` in their name are allocation 177 routines and objects returned from those functions must be freed with the 178 corresponding `*_free` function. For example, list objects returned from 179 `list_new` should be freed with `list_free` and no other freeing routine. 180 181 ### Asserts 182 Use `CHECK` liberally throughout the code to enforce invariants. Assertions 183 should not have any side-effects and should be used to detect programming logic 184 errors. Please do not use `assert`. 185 186 At minimum, every function should assert expectations on its arguments. The 187 following example demonstrates the kinds of assertions one should make on 188 function arguments. 189 ``` 190 size_t open_and_read_file(const char *filename, void *target_buffer, size_t max_bytes) { 191 CHECK(filename != NULL); 192 CHECK(filename[0] != '\0'); 193 CHECK(target_buffer != NULL); 194 CHECK(max_bytes > 0); 195 196 // function implementation begins here 197 } 198 ``` 199 200 ## Header files 201 In general, every source file (`.c` or `.cpp`) in a `src/` directory should 202 have a corresponding header (`.h`) in the `include/` directory. 203 204 ### Template header file 205 ``` 206 [copyright header] 207 208 #pragma once 209 210 #include <system/a.h> 211 #include <system/b.h> 212 213 #include "subsystem/include/a.h" 214 #include "subsystem/include/b.h" 215 216 typedef struct alarm_t alarm_t; 217 typedef struct list_t list_t; 218 219 // This comment describes the following function. It is not a structured 220 // comment, it's English prose. Function arguments can be referred to as 221 // |param|. This function returns true if a new object was created, false 222 // otherwise. 223 bool template_new(const list_t *param); 224 225 // Each public function must have a comment describing its semantics. In 226 // particular, edge cases, and whether a pointer argument may or may not be 227 // NULL. 228 void template_use_alarm(alarm_t *alarm); 229 ``` 230 231 ### License header 232 Each header file must begin with the following Apache 2.0 License with `<year>` 233 and `<owner>` replaced with the year in which the file was authored and the 234 owner of the copyright, respectively. 235 ``` 236 /****************************************************************************** 237 * 238 * Copyright <year> <owner> 239 * 240 * Licensed under the Apache License, Version 2.0 (the "License"); 241 * you may not use this file except in compliance with the License. 242 * You may obtain a copy of the License at: 243 * 244 * http://www.apache.org/licenses/LICENSE-2.0 245 * 246 * Unless required by applicable law or agreed to in writing, software 247 * distributed under the License is distributed on an "AS IS" BASIS, 248 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. 249 * See the License for the specific language governing permissions and 250 * limitations under the License. 251 * 252 ******************************************************************************/ 253 ``` 254 255 ### Include guard 256 After the license header, each header file must contain the include guard: 257 ``` 258 #pragma once 259 ``` 260 This form is used over traditional `#define`-based include guards as it is less 261 error-prone, doesn't pollute the global namespace, is more compact, and can 262 result in faster compilation. 263 264 ## Formatting 265 Code formatting is done automatically using clang-format. 266 267 The style file is located at the root of the source tree in .clang-format. The 268 -style=file option instructs clang-format to look for this file. You may find 269 clang-format --help useful for more advanced usage. The [Online clang-format 270 Documentation](http://clang.llvm.org/docs/ClangFormat.html) can also be helpful. 271 272 `clang-format -style=file -i path_to_files/filename_or_*` 273 274 ### My Patch Doesn't Apply Anymore! 275 Choose one of the options below. The fewer patches that have been applied to 276 the tree since the formatting change was applied, the better. In this short 277 guide, commands you type will be marked as `code`, with output in *italics*. 278 279 #### Option 1: The Best Case 280 Use this option if your patch touches only a few files with few intermediate 281 patches. 282 283 ##### Find the formatting patch 284 285 `git log --oneline path_to_files/filename_or_* | grep clang-format | head -n 5` 286 287 **15ce1bd** subtree: Apply **clang-format** for the first time* 288 289 ##### Revert the formatting patch 290 291 `git revert HASH -n` 292 293 (Replace HASH with 15ce1bd in this example.) 294 295 ##### Check for conflicts with your patch 296 297 `git status | grep both.modified` 298 299 If this list contains files modified by your patch, you should give up 300 301 `git revert --abort` 302 303 and try a different method. 304 305 If this list contains files not modified by your patch, you should unstage them 306 307 `git reset HEAD both_modified_file` 308 309 and remove their changes 310 311 `git checkout both_modified_file` 312 313 ##### Apply your patch 314 315 `git cherry-pick your_patch_that_used_to_apply_cleanly` 316 317 ##### Reformat the code 318 319 `clang-format -style=file -i path_to_files/filename_or_*` 320 321 ##### Commit the code that your patch touched 322 323 `git add path_to_files/filename_or_*` 324 325 `git commit --amend` 326 327 ##### Clean up any other files 328 329 `git checkout .` 330 331 ##### Review your new patch 332 333 `git diff` 334 335 #### Option 2: Reformat your patch 336 337 ##### Start with a tree with your patch applied to the tip of tree 338 339 `git log --oneline | head -n 1` 340 341 **dc5f0e2** Unformatted but vital patch* 342 343 (**Remember the HASH from this step**) 344 345 ##### Create a new branch starting from the first unrelated patch 346 347 `git checkout HEAD^ -b reformat_my_patch_branch` 348 349 `git log --oneline | head -n 1` 350 351 **15ce1bd** First Unrelated patch* 352 353 ##### Reformat the code 354 355 `clang-format -style=file -i path_to_files/filename_or_*` 356 357 ##### Commit your temporary formatting patch 358 359 `git add path_to_files/filename_or_*` 360 361 `git commit -m tmp_format_patch` 362 363 ##### Revert your temporary formatting patch (**Bear with me!**) 364 365 `git revert HEAD --no-edit` 366 367 ##### Apply your patch 368 369 `git cherry-pick HASH` 370 371 (The HASH of your patch, dc5f0e2 in this case) 372 373 ##### Reformat the code 374 375 `clang-format -style=file -i path_to_files/filename_or_*` 376 377 ##### Commit your second temporary formatting patch 378 379 `git add path_to_files/filename_or_*` 380 381 `git commit -m tmp_format_patch_2` 382 383 ##### Check to see that everything looks right 384 385 `git log --oneline | head -n 5` 386 387 *04c97cf tmp_format_patch_2* 388 389 *cf8560c Unformatted but vital patch* 390 391 *04c97cf Revert "tmp_format_patch"* 392 393 *d66bb6f tmp_format_patch* 394 395 *15ce1bd First Unrelated patch* 396 397 ##### Squash the last three patches with git rebase 398 399 `git rebase -i HEAD^^^` 400 401 *pick 04c97cf tmp_format_patch_2* 402 403 *squash cf8560c Unformatted but vital patch* 404 405 *squash 04c97cf Revert "tmp_format_patch"* 406 407 ##### Remember to edit the commit message! 408 409 `clang-format -style=file -i path_to_files/filename_or_*` 410 411 ##### Check to see that everything looks right 412 413 `git log --oneline | head -n 2` 414 415 *523078f Reformatted vital patch* 416 417 *d66bb6f tmp_format_patch* 418 419 ##### Review your new patch 420 421 `git show` 422 423 ##### Checkout the current tree and cherry pick your reformatted patch! 424 425 `git checkout aosp/master` 426 427 `git cherry-pick HASH` 428 429 (HASH is 523078f in this example) 430
