root/trunk/doc/VBox-CodingGuidelines.cpp
| Revision 9396, 16.0 kB (checked in by vboxsync, 3 months ago) | |
|---|---|
| |
| Line | |
|---|---|
| 1 | /** @file |
| 2 | * |
| 3 | * VBox - Coding Guidelines. |
| 4 | */ |
| 5 | |
| 6 | /* |
| 7 | * Copyright (C) 2006-2007 Sun Microsystems, Inc. |
| 8 | * |
| 9 | * This file is part of VirtualBox Open Source Edition (OSE), as |
| 10 | * available from http://www.virtualbox.org. This file is free software; |
| 11 | * you can redistribute it and/or modify it under the terms of the GNU |
| 12 | * General Public License (GPL) as published by the Free Software |
| 13 | * Foundation, in version 2 as it comes in the "COPYING" file of the |
| 14 | * VirtualBox OSE distribution. VirtualBox OSE is distributed in the |
| 15 | * hope that it will be useful, but WITHOUT ANY WARRANTY of any kind. |
| 16 | * |
| 17 | * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa |
| 18 | * Clara, CA 95054 USA or visit http://www.sun.com if you need |
| 19 | * additional information or have any questions. |
| 20 | */ |
| 21 | |
| 22 | /** @page pg_vbox_guideline VBox Coding Guidelines |
| 23 | * |
| 24 | * The VBox Coding guidelines are followed by all of VBox with the exception of |
| 25 | * the GUI and qemu. The GUI is using something close to the Qt style. Qemu is |
| 26 | * using whatever the frenchman does. |
| 27 | * |
| 28 | * There are a few compulsory rules and a bunch of optional ones. The following |
| 29 | * sections will describe these in details. In addition there is a section of |
| 30 | * Subversion 'rules'. |
| 31 | * |
| 32 | * |
| 33 | * |
| 34 | * @section sec_vbox_guideline_compulsory Compulsory |
| 35 | * |
| 36 | * |
| 37 | * - Use RT and VBOX types. |
| 38 | * |
| 39 | * - Use Runtime functions. |
| 40 | * |
| 41 | * - Use the standard bool, uintptr_t, intptr_t and [u]int[1-9+]_t types. |
| 42 | * |
| 43 | * - Avoid using plain unsigned and int. |
| 44 | * |
| 45 | * - Use static wherever possible. This makes the namespace less polluted |
| 46 | * and avoid nasty name clash problems which can occure, especially on |
| 47 | * Unix like systems. (1) |
| 48 | * |
| 49 | * - Public names are on the form Domain[Subdomain[]]Method using mixed |
| 50 | * casing to mark the words. The main domain is all uppercase. |
| 51 | * (Think like java, mapping domain and subdomain to packages/classes.) |
| 52 | * |
| 53 | * - Public names are always declared using the appropriate DECL macro. (2) |
| 54 | * |
| 55 | * - Internal names starts with a lowercased main domain. |
| 56 | * |
| 57 | * - Defines are all uppercase and separate words with underscore. |
| 58 | * This applies to enum values too. |
| 59 | * |
| 60 | * - Typedefs are all uppercase and contain no underscores to distinguish |
| 61 | * them from defines. |
| 62 | * |
| 63 | * - Pointer typedefs start with 'P'. If pointer to const value then 'PC'. |
| 64 | * |
| 65 | * - Function typedefs start with 'FN'. If pointer to one then 'PFN'. |
| 66 | * |
| 67 | * - All files are case sensitive. |
| 68 | * |
| 69 | * - Slashes are unix slashes ('/') runtime converts when necessary. |
| 70 | * |
| 71 | * - char strings are UTF-8. |
| 72 | * |
| 73 | * - All functions returns VBox status codes. There are three general exceptions |
| 74 | * from this: |
| 75 | * -# Predicate functions. These are function which are boolean in nature |
| 76 | * and usage. They return bool. The function name will include |
| 77 | * 'Has', 'Is' or similar. |
| 78 | * -# Functions which by nature cannot possibly fail. |
| 79 | * These return void. |
| 80 | * -# "Get"-functions which return what they ask for. |
| 81 | * A get function becomes a "Query" function if there is any |
| 82 | * doubt about getting what is ask for. |
| 83 | * |
| 84 | * - VBox status codes have three subdivisions: |
| 85 | * -# Errors, which are VERR_ prefixed and negative. |
| 86 | * -# Warnings, which are VWRN_ prefixed and positive. |
| 87 | * -# Informational, which are VINF_ prefixed and positive. |
| 88 | * |
| 89 | * - Platform/OS operation are generalized and put in the IPRT. |
| 90 | * |
| 91 | * - Other useful constructs are also put in the IPRT. |
| 92 | * |
| 93 | * - The code shall not cause compiler warnings. Check this with on ALL |
| 94 | * the platforms. |
| 95 | * |
| 96 | * - All files have file headers with $Id and a file tag which describes |
| 97 | * the file in a sentence or two. |
| 98 | * Note: Remember to enable keyword expansion when adding files to svn. |
| 99 | * |
| 100 | * - All public functions are fully documented in Doxygen style using the |
| 101 | * javadoc dialect (using the 'at' insdead of the 'slash' as commandprefix.) |
| 102 | * |
| 103 | * - All structures in header files are described, including all of their |
| 104 | * members. |
| 105 | * |
| 106 | * - All modules have a documentation 'page' in the main source file which |
| 107 | * describes the intent and actual implementation. |
| 108 | * |
| 109 | * - Code which is doing things that are not immediatly comprehendable |
| 110 | * shall include explanatory comments! |
| 111 | * |
| 112 | * - Documentation and comments are kept up to date. |
| 113 | * |
| 114 | * - Headers in /include/VBox shall not contain any slash-slash C++ comments, |
| 115 | * only ansi C comments! |
| 116 | * |
| 117 | * |
| 118 | * (1) It is common practice on Unix to have a single symbol namespace for an |
| 119 | * entire process. If one is careless symbols might be resolved in a |
| 120 | * different way that one expects, leading to weird problems. |
| 121 | * |
| 122 | * (2) This is common practice among most projects dealing with modules in |
| 123 | * shared libraries. The Windows / PE __declspect(import) and |
| 124 | * __declspect(export) constructs are the main reason for this. |
| 125 | * OTH, we do perhaps have a bit too detailed graining of this in VMM... |
| 126 | * |
| 127 | * |
| 128 | * |
| 129 | * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit |
| 130 | * |
| 131 | * Here are some amendments which address 64-bit vs. 32-bit portability issues. |
| 132 | * |
| 133 | * Some facts first: |
| 134 | * |
| 135 | * - On 64-bit Windows the type long remains 32-bit. On nearly all other |
| 136 | * 64-bit platforms long is 64-bit. |
| 137 | * |
| 138 | * - On all 64-bit platforms we care about, int is 32-bit, short is 16 bit |
| 139 | * and char is 8-bit. |
| 140 | * (I don't know about any platforms yet where this isn't true.) |
| 141 | * |
| 142 | * - size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on |
| 143 | * 64-bit platforms. (These are 32-bit on 32-bit platforms.) |
| 144 | * |
| 145 | * - There is no inline assembly support in the 64-bit Microsoft compilers. |
| 146 | * |
| 147 | * |
| 148 | * Now for the guidelines: |
| 149 | * |
| 150 | * - Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a |
| 151 | * pointer to integer. Use uintptr_t or intptr_t. If you have to use |
| 152 | * NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR. |
| 153 | * |
| 154 | * - RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__, |
| 155 | * __WIN64__ and __WIN__ because they are all deprecated and schedule |
| 156 | * for removal (if not removed already). Do not use the compiler |
| 157 | * defined _WIN32, _WIN64, or similar either. The bitness can be |
| 158 | * determined by testing ARCH_BITS. |
| 159 | * Example: |
| 160 | * @code |
| 161 | * #ifdef RT_OS_WINDOWS |
| 162 | * // call win32/64 api. |
| 163 | * #endif |
| 164 | * #ifdef RT_OS_WINDOWS |
| 165 | * # if ARCH_BITS == 64 |
| 166 | * // call win64 api. |
| 167 | * # else // ARCH_BITS == 32 |
| 168 | * // call win32 api. |
| 169 | * # endif // ARCH_BITS == 32 |
| 170 | * #else // !RT_OS_WINDOWS |
| 171 | * // call posix api |
| 172 | * #endif // !RT_OS_WINDOWS |
| 173 | * @endcode |
| 174 | * |
| 175 | * - There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS |
| 176 | * mentioned above. Use these defines instead of any predefined |
| 177 | * compiler stuff or defines from system headers. |
| 178 | * |
| 179 | * - RT_ARCH_X86 is defined when compiling for the x86 the architecture. |
| 180 | * Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar |
| 181 | * for this purpose. |
| 182 | * |
| 183 | * - RT_ARCH_AMD64 is defined when compiling for the AMD64 the architecture. |
| 184 | * Do not use __AMD64__, __amd64__ or __x64_86__. |
| 185 | * |
| 186 | * - Take care and use size_t when you have to, esp. when passing a pointer |
| 187 | * to a size_t as a parameter. |
| 188 | * |
| 189 | * - Be wary of type promotion to (signed) integer. For example the |
| 190 | * following will cause u8 to be promoted to int in the shift, and then |
| 191 | * sign extended in the assignment 64-bit: |
| 192 | * @code |
| 193 | * uint8_t u8 = 0xfe; |
| 194 | * uint64_t u64 = u8 << 24; |
| 195 | * // u64 == 0xfffffffffe000000 |
| 196 | * @endcode |
| 197 | * |
| 198 | * |
| 199 | * |
| 200 | * @section sec_vbox_guideline_optional Optional |
| 201 | * |
| 202 | * First part is the actual coding style and all the prefixes the second part is |
| 203 | * the a bunch of good advice. |
| 204 | * |
| 205 | * |
| 206 | * @subsection sec_vbox_guideline_optional_layout The code layout |
| 207 | * |
| 208 | * - Curly brackets are not indented. |
| 209 | * |
| 210 | * - Space before the parenthesis when it comes after a C keyword. |
| 211 | * |
| 212 | * - No space between argument and parenthesis. Exception for complex |
| 213 | * expression. |
| 214 | * Example: |
| 215 | * @code |
| 216 | * if (PATMR3IsPatchGCAddr(pVM, GCPtr)) |
| 217 | * @endcode |
| 218 | * |
| 219 | * - The else of an if is always first statement on a line. (No curly |
| 220 | * stuff before it!) |
| 221 | * |
| 222 | * - else and if goes on the same line if no curly stuff is needed around the if. |
| 223 | * Example: |
| 224 | * @code |
| 225 | * if (fFlags & MYFLAGS_1) |
| 226 | * fFlags &= ~MYFLAGS_10; |
| 227 | * else if (fFlags & MYFLAGS_2) |
| 228 | * { |
| 229 | * fFlags &= ~MYFLAGS_MASK; |
| 230 | * fFlags |= MYFLAGS_5; |
| 231 | * } |
| 232 | * else if (fFlags & MYFLAGS_3) |
| 233 | * @endcode |
| 234 | * |
| 235 | * - The case is indented from the switch. |
| 236 | * |
| 237 | * - If a case needs curly brackets they contain the entire case, are not |
| 238 | * indented from the case, and the break or return is placed inside them. |
| 239 | * Example: |
| 240 | * @code |
| 241 | * switch (pCur->eType) |
| 242 | * { |
| 243 | * case PGMMAPPINGTYPE_PAGETABLES: |
| 244 | * { |
| 245 | * unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT; |
| 246 | * unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT; |
| 247 | * while (iPT-- > 0) |
| 248 | * if (pPD->a[iPDE + iPT].n.u1Present) |
| 249 | * return VERR_HYPERVISOR_CONFLICT; |
| 250 | * break; |
| 251 | * } |
| 252 | * } |
| 253 | * @endcode |
| 254 | * |
| 255 | * - In a do while construction, the while is on the same line as the |
| 256 | * closing bracket if any are used. |
| 257 | * Example: |
| 258 | * @code |
| 259 | * do |
| 260 | * { |
| 261 | * stuff; |
| 262 | * i--; |
| 263 | * } while (i > 0); |
| 264 | * @endcode |
| 265 | * |
| 266 | * - Comments are in C style. C++ style comments are used for temporary |
| 267 | * disabling a few lines of code. |
| 268 | * |
| 269 | * - Sligtly complex boolean expressions are splitt into multiple lines, |
| 270 | * putting the operators first on the line and indenting it all according |
| 271 | * to the nesting of the expression. The purpose is to make it as easy as |
| 272 | * possible to read. |
| 273 | * Example: |
| 274 | * @code |
| 275 | * if ( RT_SUCCESS(rc) |
| 276 | * || (fFlags & SOME_FLAG)) |
| 277 | * @endcode |
| 278 | * |
| 279 | * - No unnecessary parentheses in expressions (just don't over do this |
| 280 | * so that gcc / msc starts bitching). Find a correct C/C++ operator |
| 281 | * precedence table if needed. |
| 282 | * |
| 283 | * |
| 284 | * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes |
| 285 | * |
| 286 | * - The 'g_' (or 'g') prefix means a global variable, either on file or module level. |
| 287 | * |
| 288 | * - The 's_' (or 's') prefix means a static variable inside a function or class. |
| 289 | * |
| 290 | * - The 'm_' (or 'm') prefix means a class data member. |
| 291 | * |
| 292 | * - The 'p' prefix means pointer. For instance 'pVM' is pointer to VM. |
| 293 | * |
| 294 | * - The 'a' prefix means array. For instance 'aPages' could be read as array |
| 295 | * of pages. |
| 296 | * |
| 297 | * - The 'c' prefix means count. For instance 'cbBlock' could be read, count |
| 298 | * of bytes in block. |
| 299 | * |
| 300 | * - The 'off' prefix means offset. |
| 301 | * |
| 302 | * - The 'i' or 'idx' prefixes usually means index. Although the 'i' one can |
| 303 | * sometimes just mean signed integer. |
| 304 | * |
| 305 | * - The 'e' (or 'enm') prefix means enum. |
| 306 | * |
| 307 | * - The 'u' prefix usually means unsigned integer. Exceptions follows. |
| 308 | * |
| 309 | * - The 'u[1-9]+' prefix means a fixed bit size variable. Frequently used |
| 310 | * with the uint[1-9]+_t types and with bitfields. |
| 311 | * |
| 312 | * - The 'b' prefix means byte or bytes. |
| 313 | * |
| 314 | * - The 'f' prefix means flags. Flags are unsigned integers of some kind or bools. |
| 315 | * |
| 316 | * - The 'ch' prefix means a char, the (signed) char type. |
| 317 | * |
| 318 | * - The 'wc' prefix means a wide/windows char, the RTUTF16 type. |
| 319 | * |
| 320 | * - The 'uc' prefix means a Unicode Code point, the RTUNICP type. |
| 321 | * |
| 322 | * - The 'uch' prefix means unsigned char. It's rarely used. |
| 323 | * |
| 324 | * - The 'sz' prefix means zero terminated character string (array of chars). (UTF-8) |
| 325 | * |
| 326 | * - The 'wsz' prefix means zero terminated wide/windows character string (array of RTUTF16). |
| 327 | * |
| 328 | * - The 'usz' prefix means zero terminated Unicode string (array of RTUNICP). |
| 329 | * |
| 330 | * - The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback' |
| 331 | * and such like. |
| 332 | * |
| 333 | * |
| 334 | * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff |
| 335 | * |
| 336 | * - When writing code think as the reader. |
| 337 | * |
| 338 | * - When writing code think as the compiler. |
| 339 | * |
| 340 | * - When reading code think as that it's fully of bugs - find them and fix them. |
| 341 | * |
| 342 | * - Pointer within range tests like: |
| 343 | * @code |
| 344 | * if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange) |
| 345 | * @endcode |
| 346 | * Can also be written as (assuming cbRange unsigned): |
| 347 | * @code |
| 348 | * if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange) |
| 349 | * @endcode |
| 350 | * Which is shorter and potentially faster. (1) |
| 351 | * |
| 352 | * - Avoid unnecessary casting. All pointers automatically casts down to void *, |
| 353 | * at least for non class instance pointers. |
| 354 | * |
| 355 | * - It's very very bad practise to write a function larger than a |
| 356 | * screen full (1024x768) without any comprehendable and explaining comments. |
| 357 | * |
| 358 | * - More to come.... |
| 359 | * |
| 360 | * |
| 361 | * (1) Important, be very careful with the casting. In particular, note that |
| 362 | * a compiler might treat pointers as signed (IIRC). |
| 363 | * |
| 364 | * |
| 365 | * |
| 366 | * |
| 367 | * @section sec_vbox_guideline_warnings Compiler Warnings |
| 368 | * |
| 369 | * The code should when possible compile on all platforms and compilers without any |
| 370 | * warnings. That's a nice idea, however, if it means making the code harder to read, |
| 371 | * less portable, unreliable or similar, the warning should not be fixed. |
| 372 | * |
| 373 | * Some of the warnings can seem kind of innocent at first glance. So, let's take the |
| 374 | * most common ones and explain them. |
| 375 | * |
| 376 | * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare |
| 377 | * |
| 378 | * GCC says: "warning: comparison between signed and unsigned integer expressions" |
| 379 | * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch" |
| 380 | * |
| 381 | * The following example will not output what you expect: |
| 382 | @code |
| 383 | #include <stdio.h> |
| 384 | int main() |
| 385 | { |
| 386 | signed long a = -1; |
| 387 | unsigned long b = 2294967295; |
| 388 | if (a < b) |
| 389 | printf("%ld < %lu: true\n", a, b); |
| 390 | else |
| 391 | printf("%ld < %lu: false\n", a, b); |
| 392 | return 0; |
| 393 | } |
| 394 | @endcode |
| 395 | * If I understood it correctly, the compiler will convert a to an unsigned long before |
| 396 | * doing the compare. |
| 397 | * |
| 398 | * |
| 399 | * @section sec_vbox_guideline_svn Subversion Commit Rules |
| 400 | * |
| 401 | * |
| 402 | * Before checking in: |
| 403 | * |
| 404 | * - Check Tinderbox and make sure the tree is green across all platforms. If it's |
| 405 | * red on a platform, don't check in. If you want, warn in the \#vbox channel and |
| 406 | * help make the responsible person fix it. |
| 407 | * NEVER CHECK IN TO A BROKEN BUILD. |
| 408 | * |
| 409 | * - When checking in keep in mind that a commit is atomical and that the Tinderbox and |
| 410 | * developers are constantly checking out the tree. Therefore do not split up the |
| 411 | * commit unless it's into 100% indepentant parts. If you need to split it up in order |
| 412 | * to have sensible commit comments, make the sub-commits as rapid as possible. |
| 413 | * |
| 414 | * - Make sure you add an entry to the ChangeLog file. |
| 415 | * |
| 416 | * |
| 417 | * After checking in: |
| 418 | * |
| 419 | * - After checking-in, you watch Tinderbox until your check-ins clear. You do not |
| 420 | * go home. You do not sleep. You do not log out or experiment with drugs. You do |
| 421 | * not become unavailable. If you break the tree, add a comment saying that you're |
| 422 | * fixing it. If you can't fix it and need help, ask in the \#innotek channel or back |
| 423 | * out the change. |
| 424 | * |
| 425 | * (Inspired by mozilla tree rules.) |
| 426 | */ |
| 427 |
Note: See TracBrowser for help on using the browser.

