.\" Copyright (c) 2017 Apple Inc. All rights reserved. .Dd 12 January, 2018 .Dt style 3 .Os Darwin .Sh NAME .Nm style .Nd C language style guide for Darwin low-level userspace projects .Sh DESCRIPTION This style's primary objective is to be as friendly to the code review process as possible. Therefore, the style aims to ensure that code changes to the project produce diffs that are .Pp .Bl -bullet -compact -offset indent .It small .It unambiguous .It viewable in side-by-side comparison tools .El .Pp As a secondary objective, this style also aims to make code as clear as possible for an uninitiated programmer reading it. "Clever" syntactic shortcuts are actively discouraged in favor of code that is easy to understand, even if it is less concise. Coincidentally, this practice also tends to lend itself to generating more readable diffs. .Pp Like any style, consistent adherence across a project is a virtue in and of itself, resulting in less distraction for the reader. However, these guidelines should be taken as exactly that: guidelines. No style can be completely adhered to all the time. When you have convinced yourself that a deviation from the style is called for, just make sure it is for the greater good and maintains the style's aims of minimizing diffs and code complexity. .Sh GENERAL PRINCIPLES .Ss Vertical space is a commodity Scrolling vertically has always been easier than scrolling horizontally. Computer mouse manufacturers went so far as to dedicate hardware to the task of scrolling vertically when they came up with scroll wheels. Even on modern trackpads, while it is possible to scroll horizontally, it is far easier to scroll vertically. You just flick upwards. Do not be afraid to introduce extra lines of code if it will result in clearer, more human-parseable diffs when those lines are changed. .Ss Horizontal space is precious Scrolling horizontally is typically awkward, imprecise, and does not lend itself well toward reading on computers or even in print. (Academic journals frequently publish with two narrow columns per page to make reading easier, for example.) Lines should be wrapped consciously; this should not be left to the editor. A soft-wrapping scheme that looks good in your editor may not look good in someone else's editor (or with a different configuration for the same editor). .Pp Just as natural language comments are difficult to read in one, long line, so too are lines of code. Both natural languages and programming languages deserve conscious, deliberate wrapping to improve readability. .Pp Wrap at a column width narrow enough to accommodate side-by-side patch review. 80 is more likely to accommodate this, but 120 might be fine too. Pick a reasonable column width and stick to it. Think about the lines you are wrapping. If you have to wrap a line, do so in a way that is clear, and be willing to make changes to accommodate that (e.g. don't be afraid to declare a variable separately if having the declaration and assignment on the same line causes it to wrap in an unclear way). .Ss Indentation is for scope indication and nothing else Indentation's sole purpose is to indicate scope. You should not use indentation to align characters on two lines of source code (beyond, of course, aligning the first characters of each line if they are both in the same scope). .Pp Given this aspect of the style, it does not particularly matter whether the author chooses spaces or tabs for indentation, and therefore the style makes no prescription (other than to pick one and stick with it). .Pp This style also has another implication: tabs and spaces should never appear in sequence. Each line will be a series of tabs followed by the first character of code. Tabs will never appear after the initial indentation of the line. .Ss Don't require leaving the source to understand it Always think of future maintainers and document your thought process for them. Remember, a "future maintainer" might be you after you've forgotten why you did something. For non-trivial changes, you should not rely on linking to a ticket-tracking system to provide context and explanation for a piece of code. You should strive to ensure the reader of your code does not have to context-switch out of reading it in order to understand it. .Pp This is not to say that linking to external resources in code is bad, but if a change's purpose can be reasonably expressed without interrupting how the code reads and flows, just do it. You don't need to publish a whitepaper in comments, but don't just give a link or ticket number with no context. .Ss Each line of code should minimize entropy It is actually very difficult to construct a hash comparison scheme that humans can use without error consistently, and there have been successful social engineering attacks on getting humans to read two hashes that are "close enough" as identical. This means that humans need a lot of help telling the difference between two lines of text. .Pp For any expression, divide it up into fundamental atoms (variable declarations, conditionals, etc.) and then assign each of those atoms to its own line of code. If you do this, when you change a single atom, it is immediately obvious that .Em only that atom changed and nothing else did. The more atoms share lines of code, the more likely it is that changes to them will generate complex diffs that humans will have difficulty understanding. .Ss Don't assume a specific editor Assume people will be reading your code in a tool that you do not control and cannot influence. Try viewing your code in such a tool and make sure that it is understandable. If you follow the guidelines of this style, your code may appear different in another viewer (in terms of how many columns are required to display a single line), but its structure will appear identical. .Sh SPECIFIC GUIDELINES .Ss Column Width and Line Wrap 80 columns opens the door for a three-way, side-by-side comparison, but it could be impractical for a number of reasons. 120 columns should provide a nice balance, but really all that matters is that you pick a width and stick to it. .Pp When indenting a continuation line, indent over by two additional tabs. This visually separates the indented line from the next line, which may itself be indented. If there is an operator in the middle of the line, the operator should .Em not be wrapped to the continuation line. .Pp .Em Good .Bd -literal -offset indent if (some_condition && some_other_condition && yet_another_condition) { exit(0); } .Ed .Pp .Em Bad .Bd -literal -offset indent if (some_condition && some_other_condition && yet_another_condition) { exit(0); } if (some_condition && some_other_condition && yet_another_condition) { exit(0); } .Ed .Pp Notice on the good example that the .Ic exit(0) line is made obviously distinct from the indented conditions above it. It's very clear on quick visual inspection that it's not a part of the conditional checks. The .Ic && is left on the first line because, when reviewing a patch to this area, it will be immediately clear to the reviewer that that line continues to the next one. .Pp .Ss Avoid prettifying alignment Indentation is used only for indicating scope, so no consideration is given to visual alignment of equal signs, colons, etc. across multiple lines. .Pp .Em Good .Bd -literal -offset indent typedef enum { THING0 = 0, THING1 = 1, THING_THAT_IS_REALLY_LONG = 2, } thing_t; .Ed .Pp .Em Bad .Bd -literal -offset indent enum { THING0 = 0, THING1 = 1, THING_THAT_IS_REALLY_LONG = 2, }; .Ed .Pp This creates bloated diffs. If you have to re-align a ton of lines after you've added something longer, you get a bunch of whitespace diffs. So for variable declarations, enumerations, assignments, etc. just keep every line independent. .Pp There is one exception to this rule, and that is if you choose to define a flagset in terms of its raw hexadecimal values and wish to align them. In this case, it is a significant benefit to have these values aligned, and you may do so with spaces. .Pp .Em Example .Bd -literal -offset indent typedef enum { F_INIT = 0x00, F_FOO = 0x01, F_BARBAZ = 0x02, F_CAD = 0x04, F_FAD = 0x08, F_FUD = 0x10, F_FLAME = 0x20, F_FOOD = 0x40, } flag_t; .Ed .Ss Only one blank line at a time Use blank lines to separate logical chunks of code. Do not use more than one. .Ss Initialization C99 has named initializers for structures. Prefer those to initializing members one-by-one later on. Both structures and arrays should be initialized in the same style, with each element of the initializer being on its own line. This is so that when an element is added to or removed from the initialization list, that change gets its own line of diff. .Pp The exception to this is the string literal. .Pp .Em Good .Bd -literal -offset indent struct my_struct baz = { .ms_foo = 1, .ms_bar = NULL, }; char *strings[] = { "string", "other string", }; .Ed .Em Bad .Bd -literal -offset indent struct my_struct baz = { 1, NULL }; struct my_struct baz = { 1, NULL }; struct my_struct baz = { .ms_foo = 1, .ms_bar = NULL, }; .Ed .Pp The last element of an initializer list should be followed by a comma. This is so that when you add a new element to that list, it's a one-line diff rather rather than a two-line diff (one line of diff to add the .Ic , to the previous-last element, and another line of diff to add the new-last element). .Pp .Em Good .Bd -literal -offset indent enum { THING0, THING1, }; struct my_point p = { .x = 1, .y = 0, .z = 1, }; .Ed .Pp .Em Bad .Bd -literal -offset indent enum { THING0, THING1, }; enum { THING0, THING1 }; struct my_point p = { .x = 1, .y = 0, .z = 1 }; .Ed .Pp Note that, if your project requires ANSI C compliance, you should disregard this guideline, as it will not work under C89. .Ss Avoid function name overloading The .Xr clang 1 compiler supports extensions to the C language which allow for function name overloading. Name overloading generally leads to code which is difficult to read and introspect and should be avoided. .Ss Prefix `struct` members Any .Ic struct which is shared or exported should have a common prefix for each member. This helps avoid collisions with preprocessor macros. .Pp .Em Good .Bd -literal -offset indent struct foobar { int64_t fb_baz; char *fb_string; }; .Ed .Pp .Em Bad .Bd -literal -offset indent struct foobar { int64_t baz; char *string; }; .Ed .Pp .Ss Types end with `_t` A type is indicated with .Ic _t at the end of the .Ic typedef , whether the type refers to a .Ic struct , .Ic union , .Ic enum , etc. All types are indicated this way. Types are in all lower-case letters. .Pp .Em Good .Bd -literal -offset indent typedef uint64_t handle_t; typedef enum foo foo_t; typedef union bar bar_t; .Ed .Pp .Em Bad .Bd -literal -offset indent typedef uint64_t Handle; typedef enum foo foo_e; typedef union bar bar_u; .Ed .Ss Use explicitly-sized integer types Avoid integer types whose names do not indicate size, such as .Ic int or .Ic long . Instead, use the types from .Ic stdint.h (e.g. .Ic int64_t , .Ic uint32_t , etc.), which explicitly indicate size. You may use size-ambiguous integer types if an API requires it. .Ss Use `sizeof()` on variables rather than types where appropriate The .Ic sizeof() operator can take both types and variables as arguments. Where possible and relevant, always pass a variable. This ensures that if the variable's type changes, the proper size is used automatically. .Pp .Em Good .Bd -literal -offset indent uuid_t ident; memcpy(ident, buff, sizeof(ident)); .Ed .Pp .Em Bad .Bd -literal -offset indent uuid_t ident; memcpy(ident, buff, sizeof(uuid_t)); .Ed .Pp .Em IMPORTANT : When applied to a .Ic char * , .Ic sizeof() will return the width of a pointer, .Em not the size of the string literal it points to, so take care to only use .Xr strlen 3 for such cases. .Pp Relatedly, when applied to an array variable that is a parameter in a function's parameter list, .Ic sizeof() will return the width of a pointer, .Em not the size of the type. .Pp .Em Good .Bd -literal -offset indent char *string = "the quick brown fox"; size_t len = strlen(string); void foo(uuid_t u) { uuid_t u2; memcpy(u2, u, sizeof(uuid_t)); } .Ed .Pp .Em Bad .Bd -literal -offset indent char *string = "the quick brown fox"; size_t len = sizeof(string) - 1; void foo(uuid_t u) { uuid_t u2; // sizeof(u) == sizeof(void *) in this context. memcpy(u2, u, sizeof(u)); } .Ed .Ss Functions which take no parameters have a parameter list of `void` In C, an empty function parameter list means that .Em any set of parameters is acceptable. In virtually all cases where you do this, you mean to have a parameter list of .Ic void . .Pp .Em Good .Bd -literal -offset indent void foo(void) { do_a_thing_without_arguments(); } .Ed .Pp .Em Bad .Bd -literal -offset indent void foo() { do_a_thing_without_arguments(); } .Ed .Ss Preprocessor macros Preprocessor definitions are written in all-caps. Macros which are function-like may be lower-case provided they do not double-evaluate their arguments. Function-like macros that do double-evaluate their arguments should be in all-caps. .Pp .Em Good .Bd -literal -offset indent #define FOO 1 #define halt() abort() // Does not double-evaluate _a and _b such that max(i++, j) is safe. #define max(_a, _b) ({ \\ typeof(_a) a1 = (_a); \\ typeof(_b) b1 = (_b); \\ (a1 < b1 ? b1 : a1); \\ }) // Double-evaluates _a and _b, so MAX(i++, j) is not safe. #define MAX(_a, _b) ((_a) < (_b) ? (_b) : (_a)) .Ed .Pp .Em Bad .Bd -literal -offset indent #define kFoo 1 // Double-evaluates _a and _b. #define max(_a, _b) ((_a) < (_b) ? (_b) : (_a)) .Ed .Pp Where possible, you should prefer inline functions to preprocessor macros, or split a macro into a preprocessor piece and an inline function piece. .Pp .Em Example .Bd -literal -offset indent static inline void _debug_uint64_impl(const char *name, uint64_t val) { fprintf(stderr, "%s = %llu\\n", name, val); } #define debug_uint64(_v) do { \\ _debug_uint64_impl(#_v, _v); \\ } while (0) .Ed .Pp In this example, the preprocessor is used to do something that only the preprocessor can do: stringify the input variable's name. But once that work is done, the actual logging of the value is done by an inline function. This keeps the code much more readable. .Ss Preprocessor macro parameters should be distinguished Preprocessor macro expansion can run afoul of naming collisions with other variables that are in the same scope as the macro being expanded. To help avoid such collisions, parameters to preprocessor macros should have a prefix, suffix or both. Another good option is to use a .Ic _[A-Z] prefix, since it is reserved by the C standard and will not collide with preprocessor evaluation. .Pp .Em Example .Bd -literal -offset indent #define foo2(_x_) ((_x_) * 2) #define foo4(_x) ((_x) * 4) #define foo8(_X) ((_X) * 8) .Ed .Ss Preprocessor macro parameters should always be evaluated When passing a parameter to a preprocessor macro, it should always be referred to within parentheses to force evaluation. The exception is for parameters intended to be string literals. .Pp .Em Good .Bd -literal -offset indent #define add2(__x) ((__x) + 2) #define println(__fmt, ...) printf(__fmt "\\n", ## __VA_ARGS__) .Ed .Pp .Em Bad .Bd -literal -offset indent #define add2(__x) x + 2 .Ed .Ss Preprocessor directives always start at column 0 Preprocessor directives do not have scope, and therefore they always start at column zero. .Pp .Em Good .Bd -literal -offset indent if (do_loop) { for (i = 0; i < 10; i++) { #if CONFIG_FOOBAR foobar(i); #else foobaz(i); #endif } } .Ed .Pp .Em Bad .Bd -literal -offset indent if (do_loop) { for (i = 0; i < 10; i++) { #if CONFIG_FOOBAR foobar(i); #else foobaz(i); #endif } } .Ed .Ss Always reference string length directly Do not hard-code the size of a string. Use either .Ic sizeof(str) - 1 or .Ic strlen(str) . In the latter case, .Xr clang 1 is smart enough to recognize when a constant string is being passed to .Xr strlen(3) and replace the function call with the string's actual length. .Pp .Em Good .Bd -literal -offset indent char str[] = "string"; frob_string(str, sizeof(str) - 1); frob_string(str, strlen(str)); .Ed .Pp .Em Bad .Bd -literal -offset indent char str[] = "string"; frob_string(str, 6); .Ed .Ss Don't pointlessly validate inputs If you control all call sites for a function, then there is no point to validating the inputs to that function. If none of your call sites pass .Ic NULL , to a pointer parameter, for example, then the a .Ic NULL input indicates that there is state corruption in your address space. You may think that it's good to catch such corruption, but .Ic NULL is just .Em one possible invalid pointer value. What if the invalid input is .Ic 0x1 ? What if it is .Ic 0x2 ? Should you also check for those? .Pp This kind of input checking complicates code. Because it indicates state corruption, the only sensible thing to do in that situation would be to abort. But the operating system has mechanisms in place to detect the reference of an invalid resource, such as virtual memory and use-after-free detection. There is no point to you duplicating these mechanisms. .Pp Of course, you should always validate inputs .Em when they come from untrusted external sources (such as a file or IPC message), but if the inputs only ever comes from your program, you should trust them. .Pp .Em Good .Bd -literal -offset indent static foo_t * get_item(foo_t *arr, size_t idx) { return &arr[idx]; } int only_call_site(foo_t *f) { foo_t *arr = calloc(10, sizeof(arr[0])); if (!arr) { return errno; } *f = get_item(arr, 0); return 0; } .Ed .Pp .Em Bad .Bd -literal -offset indent static foo_t * get_item(foo_t *arr, size_t idx) { if (!arr) { // No point to this check since we'll abort immediately below when we // try to dereference `arr`. The crash report will have more than enough // information to diagnose the NULL pointer dereference if it ever // happens. abort(); } return &arr[idx]; } int only_call_site(foo_t *f) { foo_t *arr = calloc(10, sizeof(arr[0])); if (!arr) { return errno; } *f = get_item(arr, 0); return 0; } .Ed .Ss Abort on bad API inputs The C language provides precious few compile-time validation mechanisms, and so in many cases it is not possible to fully describe to the compiler the range of expected inputs for an API. So your API should validate input from its caller and abort on invalid input. Returning an error in such a case is pointless, since the caller probably isn't checking the return code anyway. The only sure way to get the programmer's attention is to abort the calling process with a helpful message. The .Ic os_crash routine allows you to supply such a message that the crash reporter on Darwin will display in its crash report. .Pp .Em Good .Bd -literal -offset indent uint8_t foo_a_bar(uint8_t number) { if (number > (UINT8_MAX / 2)) { os_crash("number given to foo_a_bar() too large"); } return (number * 2); } .Ed .Pp .Em Bad .Bd -literal -offset indent int foo_a_bar(uint8_t number, uint8_t *new_number) { if (number > (UINT8_MAX / 2)) { return EINVAL; } *new_number = (number * 2); return 0; } .Ed .Ss Don't mingle POSIX return codes and errors Some POSIX routines have return values that indicate whether you should check .Ic errno , and others just return the error directly. While POSIX generally documents what does what pretty well, there are lots of SPIs scattered around the system that use both conventions and aren't documented at all, leaving you to spelunk through the implementation to find out what's what. .Pp To avoid confusion, do not re-use the same variable for the return codes from these functions. If an API returns a code indicating that you should check .Ic errno , name it .Ic ret or similar. If it returns the error directly, name it .Ic error or similar and make it of type .Ic errno_t . This makes it very clear to the person reading the code that you did the work to find out how the API worked. By naming the variable you store the return value in appropriately, a reader of your code (possibly Future You) can immediately know what's going on. .Pp If you are making new API or SPI that returns an error code, make it return .Ic errno_t and do not use the global .Ic errno for communicating error information. .Pp .Em Good .Bd -literal -offset indent #include errno_t error = posix_spawn(NULL, "ls", NULL, NULL, argv, envp); switch (error) { case 0: // Handle success. break; case EACCES: // Handle "permission denied". break; } int ret = reboot(RB_AUTOBOOT); if (ret == -1) { switch (errno) { case EPERM: // Handle "permission denied". break; case EBUSY: // Handle "reboot already in progress". break; } } .Ed .Pp .Em Bad .Bd -literal -offset indent int ret = posix_spawn(NULL, "ls", NULL, NULL, argv, envp); switch (error) { case 0: // Handle success. break; case EACCES: // Handle "permission denied". break; } int error = reboot(RB_AUTOBOOT); if (error == -1) { switch (errno) { case EPERM: // Handle "permission denied". break; case EBUSY: // Handle "reboot already in progress". break; } } .Ed .Ss Avoid complex `if` statements and return distinct error codes Breaking up a single complex .Ic if statement into multiple distinct checks is both more readable and makes it possible to be more granular about handling failure cases. It also leads to smaller diffs if one of those conditions turns out to require special handling. .Pp Complex .Ic if statements are often associated with input validation and just returning an error code (usually .Ic EINVAL ) if any input is invalid. While deciding which error to return in which case is more of an art than a science, that doesn't mean you should just give up and return a single error every time there isn't an immediately obvious fit to the case you've encountered. .Pp Ideally, every case where your routine may fail should be represented by a distinct error code, but this is often not practical. Still, you should attempt to distinguish each .Em likely failure case with its own error code. The POSIX error space is fairly rich, and error descriptions are brief enough that they can be liberally interpreted. For example, .Ic ESRCH can be used to apply to any situation where a resource could not be located, not just conditions where there is literally "No such process". .Pp This isn't to say that you should never have compound conditions in an .Ic if statement, but the groupings should almost always be small, and the grouped checks should be highly likely to require change as a group when change is needed. .Pp .Em Good .Bd -literal -offset indent if (foo->f_int > 10 || foo->f_int < 5) return ERANGE; } if (!foo->f_uaddr) { return EFAULT; } if (foo->f_has_idx && foo->f_idx > 100) { return ERANGE; } if (foo->f_state != FS_INITIALIZED) { return EBUSY; } .Ed .Pp .Em Bad .Bd -literal -offset indent if (foo->f_int > 10 || foo->f_int < 5 || !foo->f_uaddr || (foo->f_has_idx && foo->f_idx > 100) || foo->f_state != FS_INITIALIZED) { return EINVAL; } .Ed .Pp See .Xr intro 2 , .Ic , and .Ic for the error codes supported on Darwin. .Ss Don't NULL-check when calling `free(3)` .Ic NULL is valid input to .Xr free 3 . It's part of the API contract. Armed with this knowledge, you can do things like avoid conditional memory calls, which are always weird. .Pp .Em Good .Bd -literal -offset indent char buff[1024]; char *ptr = buff; char *what2free = NULL; if (condition) { ptr = malloc(8); what2free = ptr; } free(what2free); .Ed .Pp .Em Bad .Bd -literal -offset indent char buff[1024]; char *ptr = buff; bool did_malloc = false; if (condition) { ptr = malloc(8); did_malloc = true; } if (did_malloc) { free(ptr); } .Ed .Ss Distinguish exported and non-exported symbols Any non-exported symbols should be prefixed with a .Ic _ . Thus any .Ic static functions, project-local interfaces, etc. should have this prefix. Exported symbols (API or SPI) should .Em not have such a prefix. .Pp .Em Good .Bd -literal -offset indent static const char *_thing = "thing"; static void _foo(void); void _project_local_interface(void); .Ed .Em Bad .Bd -literal -offset indent static const char *thing = "thing"; static void foo(void); void project_local_interface(void); .Ed .Pp Global variables should have a sensible prefix, preferably related to the project name -- e.g. globals in the .Xr libxpc 3 project are prefixed with .Ic xpc_ . .Pp You may also consider declaring a global structure which contains all of your project's shared, unexported global state. This makes it very clear when code is referencing that state. Also, if your project is a library at the libSystem layer, this is required if you are ever to adopt .Xr os_alloc_once 3 . .Pp .Em Example .Bd -literal -offset indent typedef struct _foobar_globals { uint64_t fg_global_int; char *fg_global_string; } foobar_globals_t; foobar_globals_t _g; foobar_globals_t *g = &_g; .Ed .Ss Distinguish SPIs meant for one caller Sometimes projects must create bespoke SPIs for one particular caller, and these SPIs are not considered suitable for general use. Append a suffix to these SPIs to indicate their bespokeness and the intended caller with .Ic _4caller . For example, if you add an SPI specifically for IOKit, your suffix would likely be .Ic _4IOKit . .Ss Use `#if` instead of `#ifdef` where appropriate .Ic #ifdef is to check if a token is .Em defined at all to anything. .Ic #if is to check the token's value. The C standard specifies that when a token is undefined, .Ic #if will evaluate it as .Ic 0 . When checking for features, it's almost always more appropriate to use .Ic #if since the lack of a feature could still be communicated by setting the token's value to .Ic 0 , which would pass the .Ic #ifdef check. .Ss Use Function Attributes from `` If you're on Darwin, .Ic libplatform defines a lot of nice macros for compiler attributes. Use them to decorate your functions. This gives the compiler lots more information so it can do fancy optimizations. Things like .Ic OS_NONNULL let the compiler know that a parameter should never be .Ic NULL . .Ic OS_WARN_RESULT is great for enforcing that a caller always check the return value of a function. .Pp .Ic OS_MALLOC lets the compiler know that the function returns a heap allocation, and .Ic OS_OBJECT_RETURNS_RETAINED lets ARC know that the function returns an object with a reference that the caller is responsible for releasing. .Pp You can avoid having to decorate all your pointer parameters by using .Ic OS_ASSUME_NONNULL_BEGIN and .Ic OS_ASSUME_NONNULL_END and specifically annotating variables which .Em can be .Ic NULL with the .Ic _Nullable keyword. Either approach is acceptable. .Pp Generally, use these attributes on functions which will have callers who cannot view the implementation. Putting many of these attributes on (for example) an inline function is harmless, but the compiler can reason about things like .Ic OS_NONNULL and infer it when it can view the implementation at all call sites. .Pp So as a rule of thumb, if it's in a header, decorate it appropriately. These attributes can also serve as nice implicit documentation around API and SPI. For example, if you have a decoration of .Ic OS_NONNULL1 , you don't have to spell out in the HeaderDoc that you can't pass .Ic NULL for that parameter; it'll be right there in the declaration, and the compiler will catch attempts to do so. .Ss Distinguish C function definitions from declarations In C, make the definition of a function findable and distinguishable from its declaration (if any) through regular expressions. This way, you can find the implementation of .Ic foo by doing a regex search for .Ic ^foo , and you won't get the declaration as a result. .Pp .Em Good .Bd -literal -offset indent static int foo(int bar); int foo(int bar) { return bar; } .Ed .Pp .Em Bad .Bd -literal -offset indent static int foo(int bar); int foo(int bar) { return bar; } .Ed .Pp This has the additional benefit of allowing you to change the name/parameter list of a function independently of the return type. A diff of either will not be confused with the rest of the function signature. .Ss Use HeaderDoc for API declarations Make them look nice. Include the appropriate decorations (including an explicit export attribute such as .Ic OS_EXPORT so it's very, very clear that it's intended to be API), availability attributes, and HeaderDoc. Put this stuff before the function. .Pp .Em Example .Bd -literal -offset indent /*! * @function foo * Returns `bar` and ignores another parameter. * * @param bar * The value to return. * * @param baz * The value to ignore. * * @result * The value of `bar`. This routine cannot fail. */ __API_AVAILABLE(macos(10.14), ios(12.0), tvos(12.0), watchos(5.0)) OS_EXPORT OS_WARN_RESULT OS_NONNULL2 int foo(int bar, char *baz); .Ed .Ss Comments In general, use C++/C99-style comments. But there may be good reasons to use the classic C-style comments, such as for HeaderDoc, which requires them, e.g. .Bd -literal -offset indent /*! * Documentation */ .Ed .Pp Long, top-level comments may also use classic C-style comments. .Pp C++/C99-style comments may directly follow code on the same line only if they are extremely brief. Otherwise, in general, comments and code should not share a line. .Pp Also, do not get cute with .Ic /* */ comments and embed them within code. .Pp .Em Good .Bd -literal -offset indent // Comment on what the loop does. for (i = 0; i < cnt; i++) { // some code... } /* * A top-level or very long comment. */ int ret = esoteric_spi(); // returns -1 on failure, does not set errno .Ed .Pp .Em Bad .Bd -literal -offset indent //Comment int ret = esoteric_spi(); // This SPI returns -1 on failure but does not set // errno, so here is a comment explaining that that really should be above // the line of code rather than immediately following it. foo(arg1, /* first argument */, arg2 /* second argument */); .Ed .Ss `case` and `switch` are indented at the same level .Ic case and .Ic switch belong at the same column indent because indentation indicates scope, and due to case fall-through, all cases are in the same scope -- one lower than the previous. (Unless you scope them explicitly with braces, but you should avoid doing that if at all possible.) .Pp .Em Good .Bd -literal -offset indent switch (thing) { case THING1: exit(0); break; case THING2: exit(1); break; default: __builtin_unreachable(); } .Ed .Pp .Em Bad .Bd -literal -offset indent switch (thing) { case THING1: { exit(0); break; } case THING2: { exit(1); break; } default: __builtin_unreachable(); } switch (thing) { case THING1: exit(0); break; case THING2: exit(1); break; default: { __builtin_unreachable(); } } .Ed .Ss Use typed `enum`s If you're declaring an .Ic enum , you should .Ic typedef it so the compiler can reason about valid values and know the width of the .Ic enum type if possible. The .Ic OS_ENUM macro provides the correct behavior for C, C++, and Objective-C. .Ss Initialize all variables and fail closed If you pre-declare a variable before using it, initialize it to a sane value. If this value is something like the return value of the function, initialize it to a value which indicates failure of the operation. You should .Em always do this even if there are no code paths which fail to initialize the variable later. It's just good practice, and it gives the person reading your code an indication of what ranges of values the variable is expected to hold. .Pp .Em Good .Bd -literal -offset indent int result = -1; if (success) { result = 0; } .Ed .Pp .Em Bad .Bd -literal -offset indent int result; if (success) { result = 0; } .Ed .Pp Any error variable should always be initialized to a non-success condition. In general, consider success as something that your code must .Em explicitly declare and that the absence of such a declaration indicates failure. .Pp .Em Good .Bd -literal -offset indent int error = -1; if (is_root()) { error = 0; } else { error = EPERM; } .Ed .Pp .Em Bad .Bd -literal -offset indent int error = 0; if (!is_root()) { error = EPERM; } .Ed .Pp Note that you may omit an initializer for a complex .Ic struct type (such as the .Xr stat 2 .Ic struct ) but then it is incumbent upon you to ensure that that variable is not used uninitialized except to populate it. For many .Ic struct types, you can initialize them with .Ic {0} . This will not work for structures with nested structures though. For those you can use .Xr bzero 3 or similar. .Ss Using `goto` is fine .Ic goto has gotten a bad rap, but it's probably the best way in C to do lots of sequential error handling. You don't .Em have to use .Ic goto if you don't want to, but if you do, just keep a a couple things in mind. .Pp .Bl -bullet -compact -offset indent .It Compile with .Ic -Wsometimes-uninitialized . With this warning, .Xr clang 1 will catch cases where a variable may be used uninitialized because a .Ic goto skipped the initialization. .It Never use .Ic goto as a looping construct. The C language has a few different control statements for looping and iteration. Use one of those; it's not the 70's anymore. .El .Pp These guidelines make it simple to use .Ic goto effectively while avoiding the most common pitfalls. .Ss Avoid magic Booleans Sometimes you have to pass a parameter to a function to trigger some sort of behavior. Avoid using a magic Boolean for these cases. Instead, use an invariant that describes the behavior you are triggering. .Pp .Em Good .Bd -literal -offset indent replace_spaces(string, REPLACE_TABS_TOO); replace_spaces(string, REPLACE_ONLY_SPACES); .Ed .Pp .Em Bad .Bd -literal -offset indent replace_spaces(string, true); replace_spaces(string, false); .Ed .Pp If you find yourself creating many such Boolean values for function parameters, you should seriously considering defining a set of flags and passing that as one parameter instead. .Ss Spaces around binary operators In general, avoid code that looks crunched together, especially around operators. Specifically: .Bl -bullet -compact -offset indent .It Unary operators should .Em not have spaces around them. .It Binary operators .Em should have spaces around them. .It The ternary operator .Em should have spacing around it. .El .Pp .Em Good .Bd -literal -offset indent i++; j = i + k; k += condition ? i : j; .Ed .Pp .Em Bad .Bd -literal -offset indent i ++; j=i+k k+=condition?i:j; .Ed .Ss Reserve the ternary operator for trivial cases Don't use the ternary operator to choose between complex or long expressions. Reserve it for very trivial cases that are highly unlikely to change. In general if you've found yourself putting the expressions in your usage of ternary operator on multiple lines, you should just be using an .Ic if statement. .Pp .Em Good .Bd -literal -offset indent i += condition ? j : k; .Ed .Pp .Em Bad .Bd -literal -offset indent i += (i < j && j > k || i == j) ? foo(bar, baz, 0, NULL) : frob(bar, 0, NULL, baz); .Ed .Ss Spaces around parentheses .Bl -bullet -compact -offset indent .It Put a space between the control statement and the parenthesis indicating its condition. .It Do .Em not put a space between the end of a function name and the parenthesis indicating its argument list. .It Do .Em not put spaces between any parenthesis and its following content. .El .Pp .Em Good .Bd -literal -offset indent if (condition) { do_thing(); } .Ed .Pp .Em Bad .Bd -literal -offset indent if(condition) { do_thing (); } if ( condition ) { do_thing ( argument ); } .Ed .Pp .Em Worse .Bd -literal -offset indent while( condition) { do_thing( ); } .Ed .Ss Braces and statements Always, always, always use braces for your control statements. Lack of braces can and has led to serious security issues that were missed during code review, and putting the braces there from the start means that adding new statements to that clause does not require you to also add the braces. .Pp The clause should be indented on the next line with no blank lines in between. .Pp .Em Good .Bd -literal -offset indent if (condition) { do_thing(); } while (condition) { do_thing(); } .Ed .Pp .Em Bad .Bd -literal -offset indent if (condition) do_thing(); if (condition) do_thing(); while (condition) do_thing(); while (condition) { do_thing(); } .Ed .Pp Even trivial uses of braceless .Ic if statements are problematic. Consider the following: .Pp .Em Bad .Bd -literal -offset indent if (error) i++, i++; .Ed .Pp This is admittedly contrived, but it would be likely to escape code review because it's very easy to miss that the first line ends with a .Ic , rather than a .Ic ; . Braces in .Ic if statements are sensitive enough to security that the best policy is to simply always use them, without exception. .Pp Specific rules for braces: .Bl -bullet -compact -offset indent .It .Ic else goes between two braces on the same line. .It The brace which indicates the expression associated with a control flow statement goes on the same line as that statement or the same line as the last continuation line of the statement. .It The brace which begins the definition of a .Ic struct , .Ic union , .Ic enum , etc. goes on the same line as the declaration. .It The brace concluding the expression associated with a control flow statement is aligned with the same column as that control flow statement. .It The opening brace of a function definition goes on its own line and is immediately followed by a new line. .It Control statements with empty bodies should have empty braces. .El .Pp .Em Good .Bd -literal -offset indent if (condition) { do_thing(); } else { do_other_thing(); } void function(void) { return; } struct my_struct { uint32_t thing; }; for (cur; cur; cur = cur->next) { } .Ed .Pp .Em Bad .Bd -literal -offset indent if (condition) { do_thing(); } else { do_other_thing(); } if (condition) { do_thing(); } else do_other_thing(); void function(void) { return; } struct my_struct { uint32_t thing; }; for (cur; cur; cur = cur->next) .Ed .Pp .Em Worse .Bd -literal -offset indent if (condition) { do_thing(); } void function(void) { return; } .Ed .Sh SEE ALSO .Xr style 9 , .Xr intro 2 , .Xr errno 3 , .Xr types 5 .Sh HISTORY This style was largely derived from the style that evolved through the .Xr launchd 8 , .Xr libdispatch 3 , and .Xr libxpc 3 projects.