David Benjamin | b7f3144 | 2015-01-26 20:30:05 -0500 | [diff] [blame] | 1 | BoringSSL Style Guide. |
| 2 | |
| 3 | BoringSSL usually follows the Google C++ style guide, found below. The |
| 4 | rest of this document describes differences and clarifications on top |
| 5 | of the base guide. |
| 6 | |
| 7 | https://google-styleguide.googlecode.com/svn/trunk/cppguide.html |
| 8 | |
| 9 | |
| 10 | Legacy code. |
| 11 | |
| 12 | As a derivative of OpenSSL, BoringSSL contains a lot of legacy code |
| 13 | that does not follow this style guide. Particularly where public API |
| 14 | is concerned, balance consistency within a module with the benefits of |
| 15 | a given rule. Module-wide deviations on naming should be respected |
| 16 | while integer and return value conventions take precedence over |
| 17 | consistency. |
| 18 | |
| 19 | Some modules have seen few changes, so they still retain the original |
| 20 | indentation style for now. When editing these, try to retain the |
| 21 | original style. For Emacs, doc/c-indentation.el from OpenSSL may be |
| 22 | helpful in this. |
| 23 | |
| 24 | |
| 25 | Language. |
| 26 | |
| 27 | The majority of the project is in C, so C++-specific rules in the |
| 28 | Google style guide do not apply. Support for C99 features depends on |
| 29 | our target platforms. Typically, Chromium's target MSVC is the most |
| 30 | restrictive. |
| 31 | |
| 32 | Variable declarations in the middle of a function are allowed. |
| 33 | |
| 34 | Comments should be /* C-style */ for consistency. |
| 35 | |
| 36 | When declaration pointer types, * should be placed next to the variable |
| 37 | name, not the type. So |
| 38 | |
| 39 | uint8_t *ptr; |
| 40 | |
| 41 | not |
| 42 | |
| 43 | uint8_t* ptr; |
| 44 | |
| 45 | Rather than malloc() and free(), use the wrappers OPENSSL_malloc() and |
| 46 | OPENSSL_free(). Use the standard C assert() function freely. |
| 47 | |
| 48 | For new constants, prefer enums when the values are sequential and typed |
| 49 | constants for flags. If adding values to an existing set of #defines, continue |
| 50 | with #define. |
| 51 | |
| 52 | |
| 53 | Formatting. |
| 54 | |
| 55 | Single-statement blocks are not allowed. All conditions and loops must |
| 56 | use braces: |
| 57 | |
| 58 | if (foo) { |
| 59 | do_something(); |
| 60 | } |
| 61 | |
| 62 | not |
| 63 | |
| 64 | if (foo) |
| 65 | do_something(); |
| 66 | |
| 67 | |
| 68 | Integers. |
| 69 | |
| 70 | Prefer using explicitly-sized integers where appropriate rather than |
| 71 | generic C ones. For instance, to represent a byte, use uint8_t, not |
| 72 | unsigned char. Likewise, represent a two-byte field as uint16_t, not |
| 73 | unsigned short. |
| 74 | |
| 75 | Sizes are represented as size_t. |
| 76 | |
| 77 | Within a struct that is retained across the lifetime of an SSL |
| 78 | connection, if bounds of a size are known and it's easy, use a smaller |
| 79 | integer type like uint8_t. This is a "free" connection footprint |
| 80 | optimization for servers. Don't make code significantly more complex |
| 81 | for it, and do still check the bounds when passing in and out of the |
| 82 | struct. This narrowing should not propagate to local variables and |
| 83 | function parameters. |
| 84 | |
| 85 | When doing arithmetic, account for overflow conditions. |
| 86 | |
| 87 | Except with platform APIs, do not use ssize_t. MSVC lacks it, and |
| 88 | prefer out-of-band error signaling for size_t (see Return values). |
| 89 | |
| 90 | |
| 91 | Naming. |
| 92 | |
David Benjamin | b180ee9 | 2015-02-19 13:21:09 -0500 | [diff] [blame] | 93 | Follow Google naming conventions in C++ files. In C files, use the |
| 94 | following naming conventions for consistency with existing OpenSSL and C |
| 95 | styles: |
| 96 | |
David Benjamin | b7f3144 | 2015-01-26 20:30:05 -0500 | [diff] [blame] | 97 | Define structs with typedef named TYPE_NAME. The corresponding struct |
| 98 | should be named struct type_name_st. |
| 99 | |
| 100 | Name public functions as MODULE_function_name, unless the module |
| 101 | already uses a different naming scheme for legacy reasons. The module |
| 102 | name should be a type name if the function is a method of a particular |
| 103 | type. |
| 104 | |
| 105 | Some types are allocated within the library while others are |
| 106 | initialized into a struct allocated by the caller, often on the |
| 107 | stack. Name these functions TYPE_NAME_new/TYPE_NAME_free and |
| 108 | TYPE_NAME_init/TYPE_NAME_cleanup, respectively. |
| 109 | |
| 110 | If a variable is the length of a pointer value, it has the suffix |
| 111 | _len. An output parameter is named out or has an out_ prefix. For |
| 112 | instance, For instance: |
| 113 | |
| 114 | uint8_t *out, |
| 115 | size_t *out_len, |
| 116 | const uint8_t *in, |
| 117 | size_t in_len, |
| 118 | |
| 119 | Name public headers like include/openssl/evp.h with header guards like |
| 120 | OPENSSL_HEADER_EVP_H. Name internal headers like crypto/ec/internal.h |
| 121 | with header guards like OPENSSL_HEADER_EC_INTERNAL_H. |
| 122 | |
| 123 | Name enums like unix_hacker_t. For instance: |
| 124 | |
| 125 | enum should_free_handshake_buffer_t { |
| 126 | free_handshake_buffer, |
| 127 | dont_free_handshake_buffer, |
| 128 | }; |
| 129 | |
| 130 | |
| 131 | Return values. |
| 132 | |
| 133 | As even malloc may fail in BoringSSL, the vast majority of functions |
| 134 | will have a failure case. Functions should return int with one on |
| 135 | success and zero on error. Do not overload the return value to both |
| 136 | signal success/failure and output an integer. For example: |
| 137 | |
| 138 | OPENSSL_EXPORT int CBS_get_u16(CBS *cbs, uint16_t *out); |
| 139 | |
| 140 | If a function needs more than a true/false result code, define an enum |
| 141 | rather than arbitrarily assigning meaning to int values. |
| 142 | |
| 143 | If a function outputs a pointer to an object on success and there are no |
| 144 | other outputs, return the pointer directly and NULL on error. |
| 145 | |
| 146 | |
| 147 | Parameters. |
| 148 | |
| 149 | Where not constrained by legacy code, parameter order should be: |
| 150 | |
| 151 | 1. context parameters |
| 152 | 2. output parameters |
| 153 | 3. input parameters |
| 154 | |
| 155 | For example, |
| 156 | |
| 157 | /* CBB_add_asn sets |*out_contents| to a |CBB| into which the contents of an |
| 158 | * ASN.1 object can be written. The |tag| argument will be used as the tag for |
| 159 | * the object. It returns one on success or zero on error. */ |
| 160 | OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag); |
| 161 | |
| 162 | |
| 163 | Documentation. |
| 164 | |
| 165 | All public symbols must have a documentation comment in their header |
| 166 | file. The style is based on that of Go. The first sentence begins with |
| 167 | the symbol name, optionally prefixed with "A" or "An". Apart from the |
| 168 | initial mention of symbol, references to other symbols or parameter |
David Benjamin | b180ee9 | 2015-02-19 13:21:09 -0500 | [diff] [blame] | 169 | names should be surrounded by |pipes|. |
| 170 | |
| 171 | Documentation should be concise but completely describe the exposed |
| 172 | behavior of the function. Pay special note to success/failure behaviors |
| 173 | and caller obligations on object lifetimes. If this sacrifices |
| 174 | conciseness, consider simplifying the function's behavior. |
David Benjamin | b7f3144 | 2015-01-26 20:30:05 -0500 | [diff] [blame] | 175 | |
| 176 | /* EVP_DigestVerifyUpdate appends |len| bytes from |data| to the data which |
| 177 | * will be verified by |EVP_DigestVerifyFinal|. It returns one on success and |
| 178 | * zero otherwise. */ |
| 179 | OPENSSL_EXPORT int EVP_DigestVerifyUpdate(EVP_MD_CTX *ctx, const void *data, |
| 180 | size_t len); |
| 181 | |
| 182 | Explicitly mention any surprising edge cases or deviations from common |
| 183 | return value patterns in legacy functions. |
| 184 | |
| 185 | /* RSA_private_encrypt encrypts |flen| bytes from |from| with the private key in |
| 186 | * |rsa| and writes the encrypted data to |to|. The |to| buffer must have at |
| 187 | * least |RSA_size| bytes of space. It returns the number of bytes written, or |
| 188 | * -1 on error. The |padding| argument must be one of the |RSA_*_PADDING| |
| 189 | * values. If in doubt, |RSA_PKCS1_PADDING| is the most common. |
| 190 | * |
| 191 | * WARNING: this function is dangerous because it breaks the usual return value |
| 192 | * convention. Use |RSA_sign_raw| instead. */ |
| 193 | OPENSSL_EXPORT int RSA_private_encrypt(int flen, const uint8_t *from, |
| 194 | uint8_t *to, RSA *rsa, int padding); |
| 195 | |
| 196 | Document private functions in their internal.h header or, if static, |
| 197 | where defined. |