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