+.\" 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 <sys/types.h>
+
+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 <sys/errno.h> ,
+and
+.Ic <os/error.h>
+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 `<os/base.h>`
+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.
projects / apple / libc.git / commitdiff
