[19010] | 1 | /* $Id: VBox-CodingGuidelines.cpp 103312 2024-02-12 13:07:59Z vboxsync $ */
|
---|
[6303] | 2 | /** @file
|
---|
| 3 | * VBox - Coding Guidelines.
|
---|
| 4 | */
|
---|
| 5 |
|
---|
| 6 | /*
|
---|
[98103] | 7 | * Copyright (C) 2006-2023 Oracle and/or its affiliates.
|
---|
[6303] | 8 | *
|
---|
[96407] | 9 | * This file is part of VirtualBox base platform packages, as
|
---|
| 10 | * available from https://www.virtualbox.org.
|
---|
| 11 | *
|
---|
| 12 | * This program is free software; you can redistribute it and/or
|
---|
| 13 | * modify it under the terms of the GNU General Public License
|
---|
| 14 | * as published by the Free Software Foundation, in version 3 of the
|
---|
| 15 | * License.
|
---|
| 16 | *
|
---|
| 17 | * This program is distributed in the hope that it will be useful, but
|
---|
| 18 | * WITHOUT ANY WARRANTY; without even the implied warranty of
|
---|
| 19 | * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
---|
| 20 | * General Public License for more details.
|
---|
| 21 | *
|
---|
| 22 | * You should have received a copy of the GNU General Public License
|
---|
| 23 | * along with this program; if not, see <https://www.gnu.org/licenses>.
|
---|
| 24 | *
|
---|
| 25 | * SPDX-License-Identifier: GPL-3.0-only
|
---|
[6303] | 26 | */
|
---|
| 27 |
|
---|
| 28 | /** @page pg_vbox_guideline VBox Coding Guidelines
|
---|
| 29 | *
|
---|
[72791] | 30 | * The compulsory sections of these guidelines are to be followed in all of the
|
---|
| 31 | * VBox sources. Please note that local guidelines in parts of the VBox source
|
---|
| 32 | * tree may promote the optional ones to compulsory status. The VBox tree also
|
---|
| 33 | * contains some 3rd party sources where it is good to follow the local coding
|
---|
| 34 | * style while keeping these guidelines in mind.
|
---|
[6303] | 35 | *
|
---|
[72791] | 36 | * Contents:
|
---|
| 37 | * - @ref sec_vbox_guideline_compulsory
|
---|
| 38 | * - @ref sec_vbox_guideline_compulsory_sub64
|
---|
| 39 | * - @ref sec_vbox_guideline_compulsory_cppmain
|
---|
| 40 | * - @ref sec_vbox_guideline_compulsory_cppqtgui
|
---|
| 41 | * - @ref sec_vbox_guideline_compulsory_xslt
|
---|
| 42 | * - @ref sec_vbox_guideline_compulsory_doxygen
|
---|
| 43 | * - @ref sec_vbox_guideline_compulsory_guest
|
---|
| 44 | * - @ref sec_vbox_guideline_optional
|
---|
| 45 | * - @ref sec_vbox_guideline_optional_layout
|
---|
| 46 | * - @ref sec_vbox_guideline_optional_prefix
|
---|
| 47 | * - @ref sec_vbox_guideline_optional_misc
|
---|
| 48 | * - @ref sec_vbox_guideline_warnings
|
---|
| 49 | * - @ref sec_vbox_guideline_warnings_signed_unsigned_compare
|
---|
| 50 | * - @ref sec_vbox_guideline_svn
|
---|
[6303] | 51 | *
|
---|
[72791] | 52 | * Local guidelines overrides:
|
---|
| 53 | * - src/VBox/VMM/: @ref pg_vmm_guideline (src/VBox/VMM/Docs-CodingGuidelines.cpp)
|
---|
| 54 | * - src/VBox/ValidationKit/: @ref pg_validationkit_guideline (src/VBox/ValidationKit/ValidationKitCodingGuidelines.cpp)
|
---|
| 55 | * - src/VBox/Runtime/: All of @ref sec_vbox_guideline_optional is mandatory.
|
---|
| 56 | * - src/VBox/Main/: @ref sec_vbox_guideline_compulsory_cppmain
|
---|
| 57 | * - src/VBox/Frontends/VirtualBox/: @ref sec_vbox_guideline_compulsory_cppqtgui
|
---|
[6303] | 58 | *
|
---|
| 59 | *
|
---|
| 60 | * @section sec_vbox_guideline_compulsory Compulsory
|
---|
| 61 | *
|
---|
[68445] | 62 | * <ul>
|
---|
[6303] | 63 | *
|
---|
[68445] | 64 | * <li> The indentation size is 4 chars.
|
---|
[30954] | 65 | *
|
---|
[68445] | 66 | * <li> Tabs are only ever used in makefiles.
|
---|
[30954] | 67 | *
|
---|
[68445] | 68 | * <li> Use RT and VBOX types.
|
---|
[6303] | 69 | *
|
---|
[68445] | 70 | * <li> Use Runtime functions.
|
---|
[6303] | 71 | *
|
---|
[68445] | 72 | * <li> Use the standard bool, uintptr_t, intptr_t and [u]int[1-9+]_t types.
|
---|
[6303] | 73 | *
|
---|
[68445] | 74 | * <li> Avoid using plain unsigned and int.
|
---|
[6303] | 75 | *
|
---|
[68445] | 76 | * <li> Use static wherever possible. This makes the namespace less polluted
|
---|
[16197] | 77 | * and avoids nasty name clash problems which can occur, especially on
|
---|
[62912] | 78 | * Unix-like systems. (1) It also simplifies locating callers when
|
---|
| 79 | * changing it (single source file vs entire VBox tree).
|
---|
[6303] | 80 | *
|
---|
[68445] | 81 | * <li> Public names are of the form Domain[Subdomain[]]Method, using mixed
|
---|
[6303] | 82 | * casing to mark the words. The main domain is all uppercase.
|
---|
| 83 | * (Think like java, mapping domain and subdomain to packages/classes.)
|
---|
| 84 | *
|
---|
[68445] | 85 | * <li> Public names are always declared using the appropriate DECL macro. (2)
|
---|
[6303] | 86 | *
|
---|
[68445] | 87 | * <li> Internal names starts with a lowercased main domain.
|
---|
[6303] | 88 | *
|
---|
[68445] | 89 | * <li> Defines are all uppercase and separate words with underscore.
|
---|
[6303] | 90 | * This applies to enum values too.
|
---|
| 91 | *
|
---|
[68445] | 92 | * <li> Typedefs are all uppercase and contain no underscores to distinguish
|
---|
[79974] | 93 | * them from defines. Alternatively, all uppercase, separate words with
|
---|
| 94 | * underscores and ending with '_T'. The latter is not allowed in IPRT.
|
---|
[6303] | 95 | *
|
---|
[68445] | 96 | * <li> Pointer typedefs start with 'P'. If pointer to const then 'PC'.
|
---|
[6303] | 97 | *
|
---|
[68445] | 98 | * <li> Function typedefs start with 'FN'. If pointer to FN then 'PFN'.
|
---|
[6303] | 99 | *
|
---|
[68445] | 100 | * <li> All files are case sensitive.
|
---|
[6303] | 101 | *
|
---|
[68445] | 102 | * <li> Slashes are unix slashes ('/') runtime converts when necessary.
|
---|
[6303] | 103 | *
|
---|
[68445] | 104 | * <li> char strings are UTF-8.
|
---|
[6303] | 105 | *
|
---|
[68445] | 106 | * <li> Strings from any external source must be treated with utmost care as
|
---|
[57833] | 107 | * they do not have to be valid UTF-8. Only trust internal strings.
|
---|
| 108 | *
|
---|
[68445] | 109 | * <li> All functions return VBox status codes. There are three general
|
---|
[16169] | 110 | * exceptions from this:
|
---|
[68445] | 111 | *
|
---|
| 112 | * <ol>
|
---|
| 113 | * <li>Predicate functions. These are function which are boolean in
|
---|
[16169] | 114 | * nature and usage. They return bool. The function name will
|
---|
| 115 | * include 'Has', 'Is' or similar.
|
---|
[68445] | 116 | * <li>Functions which by nature cannot possibly fail.
|
---|
[6303] | 117 | * These return void.
|
---|
[68445] | 118 | * <li>"Get"-functions which return what they ask for.
|
---|
[6303] | 119 | * A get function becomes a "Query" function if there is any
|
---|
| 120 | * doubt about getting what is ask for.
|
---|
[68445] | 121 | * </ol>
|
---|
[6303] | 122 | *
|
---|
[68445] | 123 | * <li> VBox status codes have three subdivisions:
|
---|
| 124 | * <ol>
|
---|
| 125 | * <li> Errors, which are VERR_ prefixed and negative.
|
---|
| 126 | * <li> Warnings, which are VWRN_ prefixed and positive.
|
---|
| 127 | * <li> Informational, which are VINF_ prefixed and positive.
|
---|
| 128 | * </ol>
|
---|
[6303] | 129 | *
|
---|
[68445] | 130 | * <li> Platform/OS operation are generalized and put in the IPRT.
|
---|
[6303] | 131 | *
|
---|
[68445] | 132 | * <li> Other useful constructs are also put in the IPRT.
|
---|
[6303] | 133 | *
|
---|
[68445] | 134 | * <li> The code shall not cause compiler warnings. Check this on ALL
|
---|
[6303] | 135 | * the platforms.
|
---|
| 136 | *
|
---|
[68445] | 137 | * <li> The use of symbols leading with single or double underscores is
|
---|
[57833] | 138 | * forbidden as that intrudes on reserved compiler/system namespace. (3)
|
---|
| 139 | *
|
---|
[68445] | 140 | * <li> All files have file headers with $Id and a file tag which describes
|
---|
[6303] | 141 | * the file in a sentence or two.
|
---|
[57833] | 142 | * Note: Use the svn-ps.cmd/svn-ps.sh utility with the -a option to add
|
---|
| 143 | * new sources with keyword expansion and exporting correctly
|
---|
| 144 | * configured.
|
---|
[6303] | 145 | *
|
---|
[68445] | 146 | * <li> All public functions are fully documented in Doxygen style using the
|
---|
[16169] | 147 | * javadoc dialect (using the 'at' instead of the 'slash' as
|
---|
| 148 | * commandprefix.)
|
---|
[6303] | 149 | *
|
---|
[68445] | 150 | * <li> All structures in header files are described, including all their
|
---|
[57833] | 151 | * members. (Doxygen style, of course.)
|
---|
[6303] | 152 | *
|
---|
[68445] | 153 | * <li> All modules have a documentation '\@page' in the main source file
|
---|
[57833] | 154 | * which describes the intent and actual implementation.
|
---|
[6303] | 155 | *
|
---|
[68445] | 156 | * <li> Code which is doing things that are not immediately comprehensible
|
---|
[16169] | 157 | * shall include explanatory comments.
|
---|
[6303] | 158 | *
|
---|
[68445] | 159 | * <li> Documentation and comments are kept up to date.
|
---|
[6303] | 160 | *
|
---|
[68445] | 161 | * <li> Headers in /include/VBox shall not contain any slash-slash C++
|
---|
[16169] | 162 | * comments, only ANSI C comments!
|
---|
[6303] | 163 | *
|
---|
[68445] | 164 | * <li> Comments on \#else indicates what begins while the comment on a
|
---|
[57833] | 165 | * \#endif indicates what ended. Only add these when there are more than
|
---|
| 166 | * a few lines (6-10) of \#ifdef'ed code, otherwise they're just clutter.
|
---|
[71660] | 167 | *
|
---|
[71705] | 168 | * <li> \#ifdefs around a single function shall be tight, i.e. no empty
|
---|
| 169 | * lines between it and the function documentation and body.
|
---|
| 170 | *
|
---|
| 171 | * <li> \#ifdefs around more than one function shall be relaxed, i.e. leave at
|
---|
| 172 | * least one line before the first function's documentation comment and
|
---|
| 173 | * one line after the end of the last function.
|
---|
| 174 | *
|
---|
[71660] | 175 | * <li> No 'else' after if block ending with 'return', 'break', or 'continue'.
|
---|
| 176 | *
|
---|
[71739] | 177 | * <li> The term 'last' is inclusive, whereas the term 'end' is exclusive.
|
---|
| 178 | *
|
---|
[71702] | 179 | * <li> Go through all of this: https://www.slideshare.net/olvemaudal/deep-c/
|
---|
| 180 | *
|
---|
[78746] | 181 | * <li> Avoid throwing exceptions, always prefer returning statuses.
|
---|
| 182 | * Crappy exception handling is rewared by a glass of water in the face.
|
---|
| 183 | *
|
---|
[103312] | 184 | * <li> Always cast bitfields members to the desired type before using them in
|
---|
| 185 | * calculations as Visual C++ and g++/clang++ may use different types.
|
---|
| 186 | *
|
---|
| 187 | * It seems like Visual C++ will use the basetype of the field, while the
|
---|
| 188 | * other two will narrow the type down to the number of bits specified
|
---|
| 189 | * and then subject it to standard type promotion which typically ends up
|
---|
| 190 | * with signed int.
|
---|
| 191 | *
|
---|
[68445] | 192 | * </ul>
|
---|
[6303] | 193 | *
|
---|
| 194 | * (1) It is common practice on Unix to have a single symbol namespace for an
|
---|
| 195 | * entire process. If one is careless symbols might be resolved in a
|
---|
| 196 | * different way that one expects, leading to weird problems.
|
---|
| 197 | *
|
---|
| 198 | * (2) This is common practice among most projects dealing with modules in
|
---|
| 199 | * shared libraries. The Windows / PE __declspect(import) and
|
---|
| 200 | * __declspect(export) constructs are the main reason for this.
|
---|
[16169] | 201 | * OTOH, we do perhaps have a bit too detailed graining of this in VMM...
|
---|
[6303] | 202 | *
|
---|
[57833] | 203 | * (3) There are guys out there grepping public sources for symbols leading with
|
---|
| 204 | * single and double underscores as well as gotos and other things
|
---|
| 205 | * considered bad practice. They'll post statistics on how bad our sources
|
---|
| 206 | * are on some mailing list, forum or similar.
|
---|
[6303] | 207 | *
|
---|
[57833] | 208 | *
|
---|
[6303] | 209 | * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit
|
---|
| 210 | *
|
---|
| 211 | * Here are some amendments which address 64-bit vs. 32-bit portability issues.
|
---|
| 212 | *
|
---|
| 213 | * Some facts first:
|
---|
| 214 | *
|
---|
[68445] | 215 | * <ul>
|
---|
| 216 | *
|
---|
| 217 | * <li> On 64-bit Windows the type long remains 32-bit. On nearly all other
|
---|
[6303] | 218 | * 64-bit platforms long is 64-bit.
|
---|
| 219 | *
|
---|
[68445] | 220 | * <li> On all 64-bit platforms we care about, int is 32-bit, short is 16 bit
|
---|
[6303] | 221 | * and char is 8-bit.
|
---|
| 222 | * (I don't know about any platforms yet where this isn't true.)
|
---|
| 223 | *
|
---|
[68445] | 224 | * <li> size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on
|
---|
[6303] | 225 | * 64-bit platforms. (These are 32-bit on 32-bit platforms.)
|
---|
| 226 | *
|
---|
[68445] | 227 | * <li> There is no inline assembly support in the 64-bit Microsoft compilers.
|
---|
[6303] | 228 | *
|
---|
[68445] | 229 | * </ul>
|
---|
[6303] | 230 | *
|
---|
| 231 | * Now for the guidelines:
|
---|
| 232 | *
|
---|
[68445] | 233 | * <ul>
|
---|
| 234 | *
|
---|
| 235 | * <li> Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a
|
---|
[63292] | 236 | * pointer to integer. Use uintptr_t or intptr_t. If you have to use
|
---|
[6303] | 237 | * NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR.
|
---|
| 238 | *
|
---|
[68445] | 239 | * <li> Avoid where ever possible the use of the types 'long' and 'unsigned
|
---|
[63292] | 240 | * long' as these differs in size between windows and the other hosts
|
---|
| 241 | * (see above).
|
---|
| 242 | *
|
---|
[68445] | 243 | * <li> RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__,
|
---|
[16169] | 244 | * __WIN64__ and __WIN__ because they are all deprecated and scheduled
|
---|
[6303] | 245 | * for removal (if not removed already). Do not use the compiler
|
---|
| 246 | * defined _WIN32, _WIN64, or similar either. The bitness can be
|
---|
| 247 | * determined by testing ARCH_BITS.
|
---|
| 248 | * Example:
|
---|
| 249 | * @code
|
---|
| 250 | * #ifdef RT_OS_WINDOWS
|
---|
| 251 | * // call win32/64 api.
|
---|
| 252 | * #endif
|
---|
| 253 | * #ifdef RT_OS_WINDOWS
|
---|
| 254 | * # if ARCH_BITS == 64
|
---|
| 255 | * // call win64 api.
|
---|
| 256 | * # else // ARCH_BITS == 32
|
---|
| 257 | * // call win32 api.
|
---|
| 258 | * # endif // ARCH_BITS == 32
|
---|
| 259 | * #else // !RT_OS_WINDOWS
|
---|
| 260 | * // call posix api
|
---|
| 261 | * #endif // !RT_OS_WINDOWS
|
---|
| 262 | * @endcode
|
---|
| 263 | *
|
---|
[68445] | 264 | * <li> There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS
|
---|
[6303] | 265 | * mentioned above. Use these defines instead of any predefined
|
---|
| 266 | * compiler stuff or defines from system headers.
|
---|
| 267 | *
|
---|
[68445] | 268 | * <li> RT_ARCH_X86 is defined when compiling for the x86 the architecture.
|
---|
[6303] | 269 | * Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar
|
---|
| 270 | * for this purpose.
|
---|
| 271 | *
|
---|
[68445] | 272 | * <li> RT_ARCH_AMD64 is defined when compiling for the AMD64 architecture.
|
---|
[6303] | 273 | * Do not use __AMD64__, __amd64__ or __x64_86__.
|
---|
| 274 | *
|
---|
[68445] | 275 | * <li> Take care and use size_t when you have to, esp. when passing a pointer
|
---|
[6303] | 276 | * to a size_t as a parameter.
|
---|
| 277 | *
|
---|
[68445] | 278 | * <li> Be wary of type promotion to (signed) integer. For example the
|
---|
[9396] | 279 | * following will cause u8 to be promoted to int in the shift, and then
|
---|
| 280 | * sign extended in the assignment 64-bit:
|
---|
| 281 | * @code
|
---|
| 282 | * uint8_t u8 = 0xfe;
|
---|
| 283 | * uint64_t u64 = u8 << 24;
|
---|
| 284 | * // u64 == 0xfffffffffe000000
|
---|
| 285 | * @endcode
|
---|
[6303] | 286 | *
|
---|
[68445] | 287 | * </ul>
|
---|
[6303] | 288 | *
|
---|
[81149] | 289 | * @subsubsection sec_vbox_guideline_compulsory_sub64_comp Comparing the GCC and MSC calling conventions
|
---|
| 290 | *
|
---|
| 291 | * GCC expects the following (cut & past from page 20 in the ABI draft 0.96):
|
---|
| 292 | *
|
---|
| 293 | * @verbatim
|
---|
| 294 | %rax temporary register; with variable arguments passes information about the
|
---|
| 295 | number of SSE registers used; 1st return register.
|
---|
| 296 | [Not preserved]
|
---|
| 297 | %rbx callee-saved register; optionally used as base pointer.
|
---|
| 298 | [Preserved]
|
---|
| 299 | %rcx used to pass 4th integer argument to functions.
|
---|
| 300 | [Not preserved]
|
---|
| 301 | %rdx used to pass 3rd argument to functions; 2nd return register
|
---|
| 302 | [Not preserved]
|
---|
| 303 | %rsp stack pointer
|
---|
| 304 | [Preserved]
|
---|
| 305 | %rbp callee-saved register; optionally used as frame pointer
|
---|
| 306 | [Preserved]
|
---|
| 307 | %rsi used to pass 2nd argument to functions
|
---|
| 308 | [Not preserved]
|
---|
| 309 | %rdi used to pass 1st argument to functions
|
---|
| 310 | [Not preserved]
|
---|
| 311 | %r8 used to pass 5th argument to functions
|
---|
| 312 | [Not preserved]
|
---|
| 313 | %r9 used to pass 6th argument to functions
|
---|
| 314 | [Not preserved]
|
---|
| 315 | %r10 temporary register, used for passing a function's static chain
|
---|
| 316 | pointer [Not preserved]
|
---|
| 317 | %r11 temporary register
|
---|
| 318 | [Not preserved]
|
---|
| 319 | %r12-r15 callee-saved registers
|
---|
| 320 | [Preserved]
|
---|
| 321 | %xmm0-%xmm1 used to pass and return floating point arguments
|
---|
| 322 | [Not preserved]
|
---|
| 323 | %xmm2-%xmm7 used to pass floating point arguments
|
---|
| 324 | [Not preserved]
|
---|
| 325 | %xmm8-%xmm15 temporary registers
|
---|
| 326 | [Not preserved]
|
---|
| 327 | %mmx0-%mmx7 temporary registers
|
---|
| 328 | [Not preserved]
|
---|
| 329 | %st0 temporary register; used to return long double arguments
|
---|
| 330 | [Not preserved]
|
---|
| 331 | %st1 temporary registers; used to return long double arguments
|
---|
| 332 | [Not preserved]
|
---|
| 333 | %st2-%st7 temporary registers
|
---|
| 334 | [Not preserved]
|
---|
| 335 | %fs Reserved for system use (as thread specific data register)
|
---|
| 336 | [Not preserved]
|
---|
| 337 | @endverbatim
|
---|
| 338 | *
|
---|
| 339 | * Direction flag is preserved as cleared.
|
---|
| 340 | * The stack must be aligned on a 16-byte boundary before the 'call/jmp' instruction.
|
---|
| 341 | *
|
---|
| 342 | * MSC expects the following:
|
---|
| 343 | * @verbatim
|
---|
| 344 | rax return value, not preserved.
|
---|
| 345 | rbx preserved.
|
---|
| 346 | rcx 1st argument, integer, not preserved.
|
---|
| 347 | rdx 2nd argument, integer, not preserved.
|
---|
| 348 | rbp preserved.
|
---|
| 349 | rsp preserved.
|
---|
| 350 | rsi preserved.
|
---|
| 351 | rdi preserved.
|
---|
| 352 | r8 3rd argument, integer, not preserved.
|
---|
| 353 | r9 4th argument, integer, not preserved.
|
---|
| 354 | r10 scratch register, not preserved.
|
---|
| 355 | r11 scratch register, not preserved.
|
---|
| 356 | r12-r15 preserved.
|
---|
| 357 | xmm0 1st argument, fp, return value, not preserved.
|
---|
| 358 | xmm1 2st argument, fp, not preserved.
|
---|
| 359 | xmm2 3st argument, fp, not preserved.
|
---|
| 360 | xmm3 4st argument, fp, not preserved.
|
---|
| 361 | xmm4-xmm5 scratch, not preserved.
|
---|
| 362 | xmm6-xmm15 preserved.
|
---|
| 363 | @endverbatim
|
---|
| 364 | *
|
---|
| 365 | * Dunno what the direction flag is...
|
---|
| 366 | * The stack must be aligned on a 16-byte boundary before the 'call/jmp' instruction.
|
---|
| 367 | *
|
---|
| 368 | *
|
---|
[21440] | 369 | * @subsection sec_vbox_guideline_compulsory_cppmain C++ guidelines for Main
|
---|
| 370 | *
|
---|
[73806] | 371 | * Since the Main API code is a large amount of C++ code, it is allowed but
|
---|
| 372 | * not required to use C++ style comments (as permanent comments, beyond the
|
---|
| 373 | * temporary use allowed by the general coding guideline). This is a weak
|
---|
| 374 | * preference, i.e. large scale comment style changes are not encouraged.
|
---|
| 375 | *
|
---|
[21440] | 376 | * Main is currently (2009) full of hard-to-maintain code that uses complicated
|
---|
| 377 | * templates. The new mid-term goal for Main is to have less custom templates
|
---|
| 378 | * instead of more for the following reasons:
|
---|
| 379 | *
|
---|
[68445] | 380 | * <ul>
|
---|
[21440] | 381 | *
|
---|
[68445] | 382 | * <li> Template code is harder to read and understand. Custom templates create
|
---|
| 383 | * territories which only the code writer understands.
|
---|
[21440] | 384 | *
|
---|
[68445] | 385 | * <li> Errors in using templates create terrible C++ compiler messages.
|
---|
[21440] | 386 | *
|
---|
[68445] | 387 | * <li> Template code is really hard to look at in a debugger.
|
---|
[21440] | 388 | *
|
---|
[68445] | 389 | * <li> Templates slow down the compiler a lot.
|
---|
[21440] | 390 | *
|
---|
[68445] | 391 | * </ul>
|
---|
[21440] | 392 | *
|
---|
[68445] | 393 | * In particular, the following bits should be considered deprecated and should
|
---|
| 394 | * NOT be used in new code:
|
---|
[21440] | 395 | *
|
---|
[68445] | 396 | * <ul>
|
---|
[21440] | 397 | *
|
---|
[68445] | 398 | * <li> everything in include/iprt/cpputils.h (auto_ref_ptr, exception_trap_base,
|
---|
| 399 | * char_auto_ptr and friends)
|
---|
[21440] | 400 | *
|
---|
[68445] | 401 | * </ul>
|
---|
| 402 | *
|
---|
| 403 | * Generally, in many cases, a simple class with a proper destructor can achieve
|
---|
| 404 | * the same effect as a 1,000-line template include file, and the code is
|
---|
| 405 | * much more accessible that way.
|
---|
| 406 | *
|
---|
| 407 | * Using standard STL templates like std::list, std::vector and std::map is OK.
|
---|
| 408 | * Exceptions are:
|
---|
| 409 | *
|
---|
| 410 | * <ul>
|
---|
| 411 | *
|
---|
| 412 | * <li> Guest Additions because we don't want to link against libstdc++ there.
|
---|
| 413 | *
|
---|
| 414 | * <li> std::string should not be used because we have iprt::MiniString and
|
---|
[21440] | 415 | * com::Utf8Str which can convert efficiently with COM's UTF-16 strings.
|
---|
| 416 | *
|
---|
[68445] | 417 | * <li> std::auto_ptr<> in general; that part of the C++ standard is just broken.
|
---|
[21440] | 418 | * Write a destructor that calls delete.
|
---|
| 419 | *
|
---|
[68445] | 420 | * </ul>
|
---|
[21440] | 421 | *
|
---|
[26352] | 422 | * @subsection sec_vbox_guideline_compulsory_cppqtgui C++ guidelines for the Qt GUI
|
---|
[21449] | 423 | *
|
---|
[26352] | 424 | * The Qt GUI is currently (2010) on its way to become more compatible to the
|
---|
| 425 | * rest of VirtualBox coding style wise. From now on, all the coding style
|
---|
| 426 | * rules described in this file are also mandatory for the Qt GUI. Additionally
|
---|
| 427 | * the following rules should be respected:
|
---|
| 428 | *
|
---|
[68445] | 429 | * <ul>
|
---|
[26352] | 430 | *
|
---|
[68445] | 431 | * <li> GUI classes which correspond to GUI tasks should be prefixed by UI (no VBox anymore)
|
---|
[26352] | 432 | *
|
---|
[68445] | 433 | * <li> Classes which extents some of the Qt classes should be prefix by QI
|
---|
[26352] | 434 | *
|
---|
[68445] | 435 | * <li> General task classes should be prefixed by C
|
---|
[26352] | 436 | *
|
---|
[68445] | 437 | * <li> Slots are prefixed by slt -> sltName
|
---|
[26352] | 438 | *
|
---|
[68445] | 439 | * <li> Signals are prefixed by sig -> sigName
|
---|
| 440 | *
|
---|
| 441 | * <li> Use Qt classes for lists, strings and so on, the use of STL classes should
|
---|
[26352] | 442 | * be avoided
|
---|
| 443 | *
|
---|
[68445] | 444 | * <li> All files like .cpp, .h, .ui, which belong together are located in the
|
---|
[26352] | 445 | * same directory and named the same
|
---|
| 446 | *
|
---|
[68445] | 447 | * </ul>
|
---|
[26352] | 448 | *
|
---|
[68445] | 449 | *
|
---|
[53944] | 450 | * @subsection sec_vbox_guideline_compulsory_xslt XSLT
|
---|
| 451 | *
|
---|
| 452 | * XSLT (eXtensible Stylesheet Language Transformations) is used quite a bit in
|
---|
| 453 | * the Main API area of VirtualBox to generate sources and bindings to that API.
|
---|
| 454 | * There are a couple of common pitfalls worth mentioning:
|
---|
| 455 | *
|
---|
[68445] | 456 | * <ul>
|
---|
| 457 | *
|
---|
| 458 | * <li> Never do repeated //interface[\@name=...] and //enum[\@name=...] lookups
|
---|
[53944] | 459 | * because they are expensive. Instead delcare xsl:key elements for these
|
---|
| 460 | * searches and do the lookup using the key() function. xsltproc uses
|
---|
| 461 | * (per current document) hash tables for each xsl:key, i.e. very fast.
|
---|
| 462 | *
|
---|
[68445] | 463 | * <li> When output type is 'text' make sure to call xsltprocNewlineOutputHack
|
---|
[53944] | 464 | * from typemap-shared.inc.xsl every few KB of output, or xsltproc will
|
---|
| 465 | * end up wasting all the time reallocating the output buffer.
|
---|
| 466 | *
|
---|
[68445] | 467 | * </ul>
|
---|
[53944] | 468 | *
|
---|
[71660] | 469 | *
|
---|
[57833] | 470 | * @subsection sec_vbox_guideline_compulsory_doxygen Doxygen Comments
|
---|
| 471 | *
|
---|
| 472 | * As mentioned above, we shall use doxygen/javadoc style commenting of public
|
---|
[71660] | 473 | * functions, typedefs, classes and such. It is mandatory to use this style
|
---|
| 474 | * everywhere!
|
---|
[57833] | 475 | *
|
---|
| 476 | * A couple of hints on how to best write doxygen comments:
|
---|
| 477 | *
|
---|
[68445] | 478 | * <ul>
|
---|
| 479 | *
|
---|
| 480 | * <li> A good class, method, function, structure or enum doxygen comment
|
---|
[57833] | 481 | * starts with a one line sentence giving a brief description of the
|
---|
| 482 | * item. Details comes in a new paragraph (after blank line).
|
---|
| 483 | *
|
---|
[68445] | 484 | * <li> Except for list generators like \@todo, \@cfgm, \@gcfgm and others,
|
---|
[57833] | 485 | * all doxygen comments are related to things in the code. So, for
|
---|
| 486 | * instance you DO NOT add a doxygen \@note comment in the middle of a
|
---|
| 487 | * because you've got something important to note, you add a normal
|
---|
| 488 | * comment like 'Note! blah, very importan blah!'
|
---|
| 489 | *
|
---|
[68445] | 490 | * <li> We do NOT use TODO/XXX/BUGBUG or similar markers in the code to flag
|
---|
[57833] | 491 | * things needing fixing later, we always use \@todo doxygen comments.
|
---|
| 492 | *
|
---|
[68445] | 493 | * <li> There is no colon after the \@todo. And it is ALWAYS in a doxygen
|
---|
[57833] | 494 | * comment.
|
---|
| 495 | *
|
---|
[68445] | 496 | * <li> The \@retval tag is used to explain status codes a method/function may
|
---|
[57833] | 497 | * returns. It is not used to describe output parameters, that is done
|
---|
| 498 | * using the \@param or \@param[out] tag.
|
---|
| 499 | *
|
---|
[68445] | 500 | * </ul>
|
---|
| 501 | *
|
---|
[57833] | 502 | * See https://www.stack.nl/~dimitri/doxygen/manual/index.html for the official
|
---|
| 503 | * doxygen documention.
|
---|
| 504 | *
|
---|
| 505 | *
|
---|
[71660] | 506 | *
|
---|
| 507 | * @subsection sec_vbox_guideline_compulsory_guest Handling of guest input
|
---|
| 508 | *
|
---|
| 509 | * First, guest input should ALWAYS be consider to be TOXIC and constructed with
|
---|
| 510 | * MALICIOUS intent! Max paranoia level!
|
---|
| 511 | *
|
---|
| 512 | * Second, when getting inputs from memory shared with the guest, be EXTREMELY
|
---|
| 513 | * careful to not re-read input from shared memory after validating it, because
|
---|
| 514 | * that will create TOCTOU problems. So, after reading input from shared memory
|
---|
[71685] | 515 | * always use the RT_UNTRUSTED_NONVOLATILE_COPY_FENCE() macro. For more details
|
---|
[71660] | 516 | * on TOCTOU: https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
|
---|
| 517 | *
|
---|
| 518 | * Thirdly, considering the recent speculation side channel issues, spectre v1
|
---|
| 519 | * in particular, we would like to be ready for future screwups. This means
|
---|
| 520 | * having input validation in a separate block of code that ends with one (or
|
---|
| 521 | * more) RT_UNTRUSTED_VALIDATED_FENCE().
|
---|
| 522 | *
|
---|
| 523 | * So the rules:
|
---|
| 524 | *
|
---|
| 525 | * <ul>
|
---|
| 526 | *
|
---|
| 527 | * <li> Mark all pointers to shared memory with RT_UNTRUSTED_VOLATILE_GUEST.
|
---|
| 528 | *
|
---|
| 529 | * <li> Copy volatile data into local variables or heap before validating
|
---|
| 530 | * them (see RT_COPY_VOLATILE() and RT_BCOPY_VOLATILE().
|
---|
| 531 | *
|
---|
| 532 | * <li> Place RT_UNTRUSTED_NONVOLATILE_COPY_FENCE() after a block copying
|
---|
| 533 | * volatile data.
|
---|
| 534 | *
|
---|
| 535 | * <li> Always validate untrusted inputs in a block ending with a
|
---|
| 536 | * RT_UNTRUSTED_VALIDATED_FENCE().
|
---|
| 537 | *
|
---|
| 538 | * <li> Use the ASSERT_GUEST_XXXX macros from VBox/AssertGuest.h to validate
|
---|
| 539 | * guest input. (Do NOT use iprt/assert.h macros.)
|
---|
| 540 | *
|
---|
| 541 | * <li> Validation of an input B may require using another input A to look up
|
---|
| 542 | * some data, in which case its necessary to insert an
|
---|
| 543 | * RT_UNTRUSTED_VALIDATED_FENCE() after validating A and before A is used
|
---|
| 544 | * for the lookup.
|
---|
| 545 | *
|
---|
| 546 | * For example A is a view identifier, idView, and B is an offset into
|
---|
| 547 | * the view's framebuffer area, offView. To validate offView (B) it is
|
---|
| 548 | * necessary to get the size of the views framebuffer region:
|
---|
| 549 | * @code
|
---|
| 550 | * uint32_t const idView = pReq->idView; // A
|
---|
| 551 | * uint32_t const offView = pReq->offView; // B
|
---|
| 552 | * RT_UNTRUSTED_NONVOLATILE_COPY_FENCE();
|
---|
| 553 | *
|
---|
| 554 | * ASSERT_GUEST_RETURN(idView < pThis->cView,
|
---|
| 555 | * VERR_INVALID_PARAMETER);
|
---|
| 556 | * RT_UNTRUSTED_VALIDATED_FENCE();
|
---|
| 557 | * const MYVIEW *pView = &pThis->aViews[idView];
|
---|
| 558 | * ASSERT_GUEST_RETURN(offView < pView->cbFramebufferArea,
|
---|
| 559 | * VERR_OUT_OF_RANGE);
|
---|
| 560 | * RT_UNTRUSTED_VALIDATED_FENCE();
|
---|
| 561 | * @endcode
|
---|
| 562 | *
|
---|
| 563 | * <li> Take care to make sure input check are not subject to integer overflow problems.
|
---|
| 564 | *
|
---|
| 565 | * For instance when validating an area, you must not just add cbDst + offDst
|
---|
| 566 | * and check against pThis->offEnd or something like that. Rather do:
|
---|
| 567 | * @code
|
---|
| 568 | * uint32_t const offDst = pReq->offDst;
|
---|
| 569 | * uint32_t const cbDst = pReq->cbDst;
|
---|
| 570 | * RT_UNTRUSTED_NONVOLATILE_COPY_FENCE();
|
---|
| 571 | *
|
---|
| 572 | * ASSERT_GUEST_RETURN( cbDst <= pThis->cbSrc
|
---|
| 573 | * && offDst < pThis->cbSrc - cbDst,
|
---|
| 574 | * VERR_OUT_OF_RANGE);
|
---|
| 575 | * RT_UNTRUSTED_VALIDATED_FENCE();
|
---|
| 576 | * @endcode
|
---|
| 577 | *
|
---|
| 578 | * <li> Input validation does not only apply to shared data cases, but also to
|
---|
| 579 | * I/O port and MMIO handlers.
|
---|
| 580 | *
|
---|
| 581 | * <li> Ditto for kernel drivers working with usermode inputs.
|
---|
| 582 | *
|
---|
| 583 | * </ul>
|
---|
| 584 | *
|
---|
| 585 | *
|
---|
| 586 | * Problem patterns:
|
---|
| 587 | * - https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
|
---|
| 588 | * - https://googleprojectzero.blogspot.de/2018/01/reading-privileged-memory-with-side.html
|
---|
| 589 | * (Variant 1 only).
|
---|
| 590 | * - https://en.wikipedia.org/wiki/Integer_overflow
|
---|
| 591 | *
|
---|
| 592 | *
|
---|
| 593 | *
|
---|
[6303] | 594 | * @section sec_vbox_guideline_optional Optional
|
---|
| 595 | *
|
---|
[30954] | 596 | * First part is the actual coding style and all the prefixes. The second part
|
---|
[16169] | 597 | * is a bunch of good advice.
|
---|
[6303] | 598 | *
|
---|
| 599 | *
|
---|
| 600 | * @subsection sec_vbox_guideline_optional_layout The code layout
|
---|
| 601 | *
|
---|
[68445] | 602 | * <ul>
|
---|
| 603 | *
|
---|
| 604 | * <li> Max line length is 130 chars. Exceptions are table-like
|
---|
[30954] | 605 | * code/initializers and Log*() statements (don't waste unnecessary
|
---|
| 606 | * vertical space on debug logging).
|
---|
[6303] | 607 | *
|
---|
[68445] | 608 | * <li> Comments should try stay within the usual 80 columns as these are
|
---|
[44115] | 609 | * denser and too long lines may be harder to read.
|
---|
[6303] | 610 | *
|
---|
[68445] | 611 | * <li> Curly brackets are not indented. Example:
|
---|
[6303] | 612 | * @code
|
---|
[30954] | 613 | * if (true)
|
---|
| 614 | * {
|
---|
| 615 | * Something1();
|
---|
| 616 | * Something2();
|
---|
| 617 | * }
|
---|
| 618 | * else
|
---|
| 619 | * {
|
---|
| 620 | * SomethingElse1().
|
---|
| 621 | * SomethingElse2().
|
---|
| 622 | * }
|
---|
| 623 | * @endcode
|
---|
| 624 | *
|
---|
[68445] | 625 | * <li> Space before the parentheses when it comes after a C keyword.
|
---|
[30954] | 626 | *
|
---|
[68445] | 627 | * <li> No space between argument and parentheses. Exception for complex
|
---|
[30954] | 628 | * expression. Example:
|
---|
| 629 | * @code
|
---|
[6303] | 630 | * if (PATMR3IsPatchGCAddr(pVM, GCPtr))
|
---|
| 631 | * @endcode
|
---|
| 632 | *
|
---|
[68445] | 633 | * <li> The else of an if is always the first statement on a line. (No curly
|
---|
[6303] | 634 | * stuff before it!)
|
---|
| 635 | *
|
---|
[68445] | 636 | * <li> else and if go on the same line if no { compound statement }
|
---|
[30954] | 637 | * follows the if. Example:
|
---|
[6303] | 638 | * @code
|
---|
| 639 | * if (fFlags & MYFLAGS_1)
|
---|
| 640 | * fFlags &= ~MYFLAGS_10;
|
---|
| 641 | * else if (fFlags & MYFLAGS_2)
|
---|
| 642 | * {
|
---|
| 643 | * fFlags &= ~MYFLAGS_MASK;
|
---|
| 644 | * fFlags |= MYFLAGS_5;
|
---|
| 645 | * }
|
---|
| 646 | * else if (fFlags & MYFLAGS_3)
|
---|
| 647 | * @endcode
|
---|
| 648 | *
|
---|
[68445] | 649 | * <li> Slightly complex boolean expressions are split into multiple lines,
|
---|
[30954] | 650 | * putting the operators first on the line and indenting it all according
|
---|
| 651 | * to the nesting of the expression. The purpose is to make it as easy as
|
---|
| 652 | * possible to read. Example:
|
---|
| 653 | * @code
|
---|
| 654 | * if ( RT_SUCCESS(rc)
|
---|
| 655 | * || (fFlags & SOME_FLAG))
|
---|
| 656 | * @endcode
|
---|
| 657 | *
|
---|
[68445] | 658 | * <li> When 'if' or 'while' statements gets long, the closing parentheses
|
---|
[30954] | 659 | * goes right below the opening parentheses. This may be applied to
|
---|
| 660 | * sub-expression. Example:
|
---|
| 661 | * @code
|
---|
| 662 | * if ( RT_SUCCESS(rc)
|
---|
| 663 | * || ( fSomeStuff
|
---|
| 664 | * && fSomeOtherStuff
|
---|
| 665 | * && fEvenMoreStuff
|
---|
| 666 | * )
|
---|
| 667 | * || SomePredicateFunction()
|
---|
| 668 | * )
|
---|
| 669 | * {
|
---|
| 670 | * ...
|
---|
| 671 | * }
|
---|
| 672 | * @endcode
|
---|
| 673 | *
|
---|
[68445] | 674 | * <li> The case is indented from the switch (to avoid having the braces for
|
---|
[30954] | 675 | * the 'case' at the same level as the 'switch' statement).
|
---|
| 676 | *
|
---|
[68445] | 677 | * <li> If a case needs curly brackets they contain the entire case, are not
|
---|
[6303] | 678 | * indented from the case, and the break or return is placed inside them.
|
---|
| 679 | * Example:
|
---|
| 680 | * @code
|
---|
| 681 | * switch (pCur->eType)
|
---|
| 682 | * {
|
---|
| 683 | * case PGMMAPPINGTYPE_PAGETABLES:
|
---|
| 684 | * {
|
---|
| 685 | * unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT;
|
---|
| 686 | * unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT;
|
---|
| 687 | * while (iPT-- > 0)
|
---|
| 688 | * if (pPD->a[iPDE + iPT].n.u1Present)
|
---|
| 689 | * return VERR_HYPERVISOR_CONFLICT;
|
---|
| 690 | * break;
|
---|
| 691 | * }
|
---|
| 692 | * }
|
---|
| 693 | * @endcode
|
---|
| 694 | *
|
---|
[68445] | 695 | * <li> In a do while construction, the while is on the same line as the
|
---|
[16169] | 696 | * closing "}" if any are used.
|
---|
[6303] | 697 | * Example:
|
---|
| 698 | * @code
|
---|
| 699 | * do
|
---|
| 700 | * {
|
---|
| 701 | * stuff;
|
---|
| 702 | * i--;
|
---|
| 703 | * } while (i > 0);
|
---|
| 704 | * @endcode
|
---|
| 705 | *
|
---|
[68445] | 706 | * <li> Comments are in C style. C++ style comments are used for temporary
|
---|
[6303] | 707 | * disabling a few lines of code.
|
---|
| 708 | *
|
---|
[68445] | 709 | * <li> No unnecessary parentheses in expressions (just don't over do this
|
---|
[6303] | 710 | * so that gcc / msc starts bitching). Find a correct C/C++ operator
|
---|
| 711 | * precedence table if needed.
|
---|
| 712 | *
|
---|
[68445] | 713 | * <li> 'for (;;)' is preferred over 'while (true)' and 'while (1)'.
|
---|
[6303] | 714 | *
|
---|
[68445] | 715 | * <li> Parameters are indented to the start parentheses when breaking up
|
---|
[30954] | 716 | * function calls, declarations or prototypes. (This is in line with
|
---|
| 717 | * how 'if', 'for' and 'while' statements are done as well.) Example:
|
---|
| 718 | * @code
|
---|
| 719 | * RTPROCESS hProcess;
|
---|
| 720 | * int rc = RTProcCreateEx(papszArgs[0],
|
---|
| 721 | * papszArgs,
|
---|
| 722 | * RTENV_DEFAULT,
|
---|
| 723 | * fFlags,
|
---|
| 724 | * NULL, // phStdIn
|
---|
| 725 | * NULL, // phStdOut
|
---|
| 726 | * NULL, // phStdErr
|
---|
| 727 | * NULL, // pszAsUser
|
---|
| 728 | * NULL, // pszPassword
|
---|
[80569] | 729 | * NULL, // pExtraData
|
---|
[30954] | 730 | * &hProcess);
|
---|
| 731 | * @endcode
|
---|
| 732 | *
|
---|
[68445] | 733 | * <li> That Dijkstra is dead is no excuse for using gotos.
|
---|
[30954] | 734 | *
|
---|
[68445] | 735 | * <li> Using do-while-false loops to avoid gotos is considered very bad form.
|
---|
[62704] | 736 | * They create hard to read code. They tend to be either too short (i.e.
|
---|
| 737 | * pointless) or way to long (split up the function already), making
|
---|
| 738 | * tracking the state is difficult and prone to bugs. Also, they cause
|
---|
| 739 | * the compiler to generate suboptimal code, because the break branches
|
---|
| 740 | * are by preferred over the main code flow (MSC has no branch hinting!).
|
---|
| 741 | * Instead, do make use the 130 columns (i.e. nested ifs) and split
|
---|
| 742 | * the code up into more functions!
|
---|
[30954] | 743 | *
|
---|
[68445] | 744 | * <li> Avoid code like
|
---|
| 745 | * @code
|
---|
| 746 | * int foo;
|
---|
| 747 | * int rc;
|
---|
| 748 | * ...
|
---|
| 749 | * rc = FooBar();
|
---|
| 750 | * if (RT_SUCCESS(rc))
|
---|
| 751 | * {
|
---|
| 752 | * foo = getFoo();
|
---|
| 753 | * ...
|
---|
| 754 | * pvBar = RTMemAlloc(sizeof(*pvBar));
|
---|
| 755 | * if (!pvBar)
|
---|
| 756 | * rc = VERR_NO_MEMORY;
|
---|
[69209] | 757 | * }
|
---|
[68445] | 758 | * if (RT_SUCCESS(rc))
|
---|
| 759 | * {
|
---|
| 760 | * buzz = foo;
|
---|
| 761 | * ...
|
---|
| 762 | * }
|
---|
| 763 | * @endcode
|
---|
| 764 | * The intention of such code is probably to save some horizontal space
|
---|
| 765 | * but unfortunately it's hard to read and the scope of certain varables
|
---|
| 766 | * (e.g. foo in this example) is not optimal. Better use the following
|
---|
| 767 | * style:
|
---|
| 768 | * @code
|
---|
| 769 | * int rc;
|
---|
| 770 | * ...
|
---|
| 771 | * rc = FooBar();
|
---|
| 772 | * if (RT_SUCCESS(rc))
|
---|
| 773 | * {
|
---|
| 774 | * int foo = getFoo();
|
---|
| 775 | * ...
|
---|
| 776 | * pvBar = RTMemAlloc(sizeof(*pvBar));
|
---|
| 777 | * if (pvBar)
|
---|
| 778 | * {
|
---|
| 779 | * buzz = foo;
|
---|
| 780 | * ...
|
---|
| 781 | * }
|
---|
| 782 | * else
|
---|
| 783 | * rc = VERR_NO_MEMORY;
|
---|
| 784 | * }
|
---|
| 785 | * @endcode
|
---|
[30954] | 786 | *
|
---|
[68445] | 787 | * </ul>
|
---|
| 788 | *
|
---|
[6303] | 789 | * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes
|
---|
| 790 | *
|
---|
[44115] | 791 | * Prefixes are meant to provide extra context clues to a variable/member, we
|
---|
| 792 | * therefore avoid using prefixes that just indicating the type if a better
|
---|
| 793 | * choice is available.
|
---|
| 794 | *
|
---|
| 795 | *
|
---|
| 796 | * The prefixes:
|
---|
| 797 | *
|
---|
[68445] | 798 | * <ul>
|
---|
[6303] | 799 | *
|
---|
[68445] | 800 | * <li> The 'g_' (or 'g') prefix means a global variable, either on file or module level.
|
---|
| 801 | *
|
---|
| 802 | * <li> The 's_' (or 's') prefix means a static variable inside a function or
|
---|
[62912] | 803 | * class. This is not used for static variables on file level, use 'g_'
|
---|
| 804 | * for those (logical, right).
|
---|
[6303] | 805 | *
|
---|
[68445] | 806 | * <li> The 'm_' (or 'm') prefix means a class data member.
|
---|
[6303] | 807 | *
|
---|
[30954] | 808 | * In new code in Main, use "m_" (and common sense). As an exception,
|
---|
| 809 | * in Main, if a class encapsulates its member variables in an anonymous
|
---|
[25113] | 810 | * structure which is declared in the class, but defined only in the
|
---|
[30954] | 811 | * implementation (like this: 'class X { struct Data; Data *m; }'), then
|
---|
| 812 | * the pointer to that struct is called 'm' itself and its members then
|
---|
| 813 | * need no prefix, because the members are accessed with 'm->member'
|
---|
| 814 | * already which is clear enough.
|
---|
[25113] | 815 | *
|
---|
[68445] | 816 | * <li> The 'a_' prefix means a parameter (argument) variable. This is
|
---|
[30954] | 817 | * sometimes written 'a' in parts of the source code that does not use
|
---|
| 818 | * the array prefix.
|
---|
[6303] | 819 | *
|
---|
[101582] | 820 | * <li> The 'p' prefix means pointer. For instance 'pVM' is pointer to VM,
|
---|
| 821 | * 'pidx' means pointer to an index and 'pv' is a generic void pointer.
|
---|
[6303] | 822 | *
|
---|
[68445] | 823 | * <li> The 'r' prefix means that something is passed by reference.
|
---|
[6303] | 824 | *
|
---|
[68445] | 825 | * <li> The 'k' prefix means that something is a constant. For instance
|
---|
[30954] | 826 | * 'enum { kStuff };'. This is usually not used in combination with
|
---|
| 827 | * 'p', 'r' or any such thing, it's main main use is to make enums
|
---|
| 828 | * easily identifiable.
|
---|
| 829 | *
|
---|
[68445] | 830 | * <li> The 'a' prefix means array. For instance 'aPages' could be read as
|
---|
[30954] | 831 | * array of pages.
|
---|
| 832 | *
|
---|
[68445] | 833 | * <li> The 'c' prefix means count. For instance 'cbBlock' could be read,
|
---|
[62705] | 834 | * count of bytes in block. (1)
|
---|
[30954] | 835 | *
|
---|
[68445] | 836 | * <li> The 'cx' prefix means width (count of 'x' units).
|
---|
[44115] | 837 | *
|
---|
[68445] | 838 | * <li> The 'cy' prefix means height (count of 'y' units).
|
---|
[44115] | 839 | *
|
---|
[68445] | 840 | * <li> The 'x', 'y' and 'z' prefix refers to the x-, y- , and z-axis
|
---|
[44115] | 841 | * respectively.
|
---|
| 842 | *
|
---|
[68445] | 843 | * <li> The 'off' prefix means offset.
|
---|
[6303] | 844 | *
|
---|
[68445] | 845 | * <li> The 'i' or 'idx' prefixes usually means index. Although the 'i' one
|
---|
[30954] | 846 | * can sometimes just mean signed integer.
|
---|
[6303] | 847 | *
|
---|
[68445] | 848 | * <li> The 'i[1-9]+' prefix means a fixed bit size variable. Frequently
|
---|
[44115] | 849 | * used with the int[1-9]+_t types where the width is really important.
|
---|
| 850 | * In most cases 'i' is more appropriate. [type]
|
---|
[30954] | 851 | *
|
---|
[68445] | 852 | * <li> The 'e' (or 'enm') prefix means enum.
|
---|
[6303] | 853 | *
|
---|
[68445] | 854 | * <li> The 'u' prefix usually means unsigned integer. Exceptions follows.
|
---|
[6303] | 855 | *
|
---|
[68445] | 856 | * <li> The 'u[1-9]+' prefix means a fixed bit size variable. Frequently
|
---|
[44115] | 857 | * used with the uint[1-9]+_t types and with bitfields where the width is
|
---|
| 858 | * really important. In most cases 'u' or 'b' (byte) would be more
|
---|
| 859 | * appropriate. [type]
|
---|
[6303] | 860 | *
|
---|
[68445] | 861 | * <li> The 'b' prefix means byte or bytes. [type]
|
---|
[6303] | 862 | *
|
---|
[68445] | 863 | * <li> The 'f' prefix means flags. Flags are unsigned integers of some kind
|
---|
[30954] | 864 | * or booleans.
|
---|
[6303] | 865 | *
|
---|
[68445] | 866 | * <li> TODO: need prefix for real float. [type]
|
---|
[6303] | 867 | *
|
---|
[68445] | 868 | * <li> The 'rd' prefix means real double and is used for 'double' variables.
|
---|
[30954] | 869 | * [type]
|
---|
[6303] | 870 | *
|
---|
[68445] | 871 | * <li> The 'lrd' prefix means long real double and is used for 'long double'
|
---|
[30954] | 872 | * variables. [type]
|
---|
[6303] | 873 | *
|
---|
[68445] | 874 | * <li> The 'ch' prefix means a char, the (signed) char type. [type]
|
---|
[6303] | 875 | *
|
---|
[68445] | 876 | * <li> The 'wc' prefix means a wide/windows char, the RTUTF16 type. [type]
|
---|
[6303] | 877 | *
|
---|
[68445] | 878 | * <li> The 'uc' prefix means a Unicode Code point, the RTUNICP type. [type]
|
---|
[6303] | 879 | *
|
---|
[68445] | 880 | * <li> The 'uch' prefix means unsigned char. It's rarely used. [type]
|
---|
[6303] | 881 | *
|
---|
[68445] | 882 | * <li> The 'sz' prefix means zero terminated character string (array of
|
---|
[30954] | 883 | * chars). (UTF-8)
|
---|
| 884 | *
|
---|
[68445] | 885 | * <li> The 'wsz' prefix means zero terminated wide/windows character string
|
---|
[30954] | 886 | * (array of RTUTF16).
|
---|
| 887 | *
|
---|
[68445] | 888 | * <li> The 'usz' prefix means zero terminated Unicode string (array of
|
---|
[30954] | 889 | * RTUNICP).
|
---|
| 890 | *
|
---|
[68445] | 891 | * <li> The 'str' prefix means C++ string; either a std::string or, in Main,
|
---|
[30954] | 892 | * a Utf8Str or, in Qt, a QString. When used with 'p', 'r', 'a' or 'c'
|
---|
| 893 | * the first letter should be capitalized.
|
---|
[25113] | 894 | *
|
---|
[68445] | 895 | * <li> The 'bstr' prefix, in Main, means a UTF-16 Bstr. When used with 'p',
|
---|
[30954] | 896 | * 'r', 'a' or 'c' the first letter should be capitalized.
|
---|
[25113] | 897 | *
|
---|
[68445] | 898 | * <li> The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback'
|
---|
[6303] | 899 | * and such like.
|
---|
| 900 | *
|
---|
[68445] | 901 | * <li> The 'psz' prefix is a combination of 'p' and 'sz' and thus means
|
---|
[30954] | 902 | * pointer to a zero terminated character string. (UTF-8)
|
---|
[6303] | 903 | *
|
---|
[68445] | 904 | * <li> The 'pcsz' prefix is used to indicate constant string pointers in
|
---|
[30954] | 905 | * parts of the code. Most code uses 'psz' for const and non-const
|
---|
[62705] | 906 | * string pointers, so please ignore this one.
|
---|
[30954] | 907 | *
|
---|
[68445] | 908 | * <li> The 'l' prefix means (signed) long. We try avoid using this,
|
---|
[30954] | 909 | * expecially with the 'LONG' types in Main as these are not 'long' on
|
---|
| 910 | * 64-bit non-Windows platforms and can cause confusion. Alternatives:
|
---|
| 911 | * 'i' or 'i32'. [type]
|
---|
| 912 | *
|
---|
[68445] | 913 | * <li> The 'ul' prefix means unsigned long. We try avoid using this,
|
---|
[30954] | 914 | * expecially with the 'ULONG' types in Main as these are not 'unsigned
|
---|
| 915 | * long' on 64-bit non-Windows platforms and can cause confusion.
|
---|
| 916 | * Alternatives: 'u' or 'u32'. [type]
|
---|
| 917 | *
|
---|
[68445] | 918 | * </ul>
|
---|
[30954] | 919 | *
|
---|
[62705] | 920 | * (1) Except in the occasional 'pcsz' prefix, the 'c' prefix is never ever
|
---|
| 921 | * used in the meaning 'const'.
|
---|
| 922 | *
|
---|
| 923 | *
|
---|
[6303] | 924 | * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff
|
---|
| 925 | *
|
---|
[68445] | 926 | * <ul>
|
---|
[6303] | 927 | *
|
---|
[68445] | 928 | * <li> When writing code think as the reader.
|
---|
[6303] | 929 | *
|
---|
[68445] | 930 | * <li> When writing code think as the compiler. (2)
|
---|
[6303] | 931 | *
|
---|
[68445] | 932 | * <li> When reading code think as if it's full of bugs - find them and fix them.
|
---|
| 933 | *
|
---|
| 934 | * <li> Pointer within range tests like:
|
---|
[6303] | 935 | * @code
|
---|
| 936 | * if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange)
|
---|
| 937 | * @endcode
|
---|
| 938 | * Can also be written as (assuming cbRange unsigned):
|
---|
| 939 | * @code
|
---|
| 940 | * if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange)
|
---|
| 941 | * @endcode
|
---|
| 942 | * Which is shorter and potentially faster. (1)
|
---|
| 943 | *
|
---|
[68445] | 944 | * <li> Avoid unnecessary casting. All pointers automatically cast down to
|
---|
[16169] | 945 | * void *, at least for non class instance pointers.
|
---|
[6303] | 946 | *
|
---|
[68445] | 947 | * <li> It's very very bad practise to write a function larger than a
|
---|
[16169] | 948 | * screen full (1024x768) without any comprehensibility and explaining
|
---|
| 949 | * comments.
|
---|
[6303] | 950 | *
|
---|
[68445] | 951 | * <li> More to come....
|
---|
[6303] | 952 | *
|
---|
[68445] | 953 | * </ul>
|
---|
[6303] | 954 | *
|
---|
| 955 | * (1) Important, be very careful with the casting. In particular, note that
|
---|
| 956 | * a compiler might treat pointers as signed (IIRC).
|
---|
| 957 | *
|
---|
[26668] | 958 | * (2) "A really advanced hacker comes to understand the true inner workings of
|
---|
| 959 | * the machine - he sees through the language he's working in and glimpses
|
---|
| 960 | * the secret functioning of the binary code - becomes a Ba'al Shem of
|
---|
| 961 | * sorts." (Neal Stephenson "Snow Crash")
|
---|
[6303] | 962 | *
|
---|
| 963 | *
|
---|
| 964 | *
|
---|
| 965 | * @section sec_vbox_guideline_warnings Compiler Warnings
|
---|
| 966 | *
|
---|
| 967 | * The code should when possible compile on all platforms and compilers without any
|
---|
| 968 | * warnings. That's a nice idea, however, if it means making the code harder to read,
|
---|
| 969 | * less portable, unreliable or similar, the warning should not be fixed.
|
---|
| 970 | *
|
---|
| 971 | * Some of the warnings can seem kind of innocent at first glance. So, let's take the
|
---|
| 972 | * most common ones and explain them.
|
---|
| 973 | *
|
---|
[21449] | 974 | *
|
---|
[6303] | 975 | * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare
|
---|
| 976 | *
|
---|
| 977 | * GCC says: "warning: comparison between signed and unsigned integer expressions"
|
---|
| 978 | * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch"
|
---|
| 979 | *
|
---|
| 980 | * The following example will not output what you expect:
|
---|
| 981 | @code
|
---|
| 982 | #include <stdio.h>
|
---|
| 983 | int main()
|
---|
| 984 | {
|
---|
| 985 | signed long a = -1;
|
---|
| 986 | unsigned long b = 2294967295;
|
---|
| 987 | if (a < b)
|
---|
| 988 | printf("%ld < %lu: true\n", a, b);
|
---|
| 989 | else
|
---|
| 990 | printf("%ld < %lu: false\n", a, b);
|
---|
| 991 | return 0;
|
---|
| 992 | }
|
---|
| 993 | @endcode
|
---|
[16169] | 994 | * If I understood it correctly, the compiler will convert a to an
|
---|
| 995 | * unsigned long before doing the compare.
|
---|
[6303] | 996 | *
|
---|
| 997 | *
|
---|
[21449] | 998 | *
|
---|
[6303] | 999 | * @section sec_vbox_guideline_svn Subversion Commit Rules
|
---|
| 1000 | *
|
---|
| 1001 | *
|
---|
| 1002 | * Before checking in:
|
---|
| 1003 | *
|
---|
[68445] | 1004 | * <ul>
|
---|
| 1005 | *
|
---|
| 1006 | * <li> Check Tinderbox and make sure the tree is green across all platforms. If it's
|
---|
[6303] | 1007 | * red on a platform, don't check in. If you want, warn in the \#vbox channel and
|
---|
| 1008 | * help make the responsible person fix it.
|
---|
| 1009 | * NEVER CHECK IN TO A BROKEN BUILD.
|
---|
| 1010 | *
|
---|
[68445] | 1011 | * <li> When checking in keep in mind that a commit is atomic and that the Tinderbox and
|
---|
[6303] | 1012 | * developers are constantly checking out the tree. Therefore do not split up the
|
---|
[16169] | 1013 | * commit unless it's into 100% independent parts. If you need to split it up in order
|
---|
[6303] | 1014 | * to have sensible commit comments, make the sub-commits as rapid as possible.
|
---|
| 1015 | *
|
---|
[68445] | 1016 | * <li> If you make a user visible change, such as fixing a reported bug,
|
---|
[16169] | 1017 | * make sure you add an entry to doc/manual/user_ChangeLogImpl.xml.
|
---|
[6303] | 1018 | *
|
---|
[68445] | 1019 | * <li> If you are adding files make sure set the right attributes.
|
---|
[21449] | 1020 | * svn-ps.sh/cmd was created for this purpose, please make use of it.
|
---|
[6303] | 1021 | *
|
---|
[68445] | 1022 | * </ul>
|
---|
[21449] | 1023 | *
|
---|
[6303] | 1024 | * After checking in:
|
---|
| 1025 | *
|
---|
[68445] | 1026 | * <ul>
|
---|
| 1027 | *
|
---|
| 1028 | * <li> After checking-in, you watch Tinderbox until your check-ins clear. You do not
|
---|
[6303] | 1029 | * go home. You do not sleep. You do not log out or experiment with drugs. You do
|
---|
| 1030 | * not become unavailable. If you break the tree, add a comment saying that you're
|
---|
| 1031 | * fixing it. If you can't fix it and need help, ask in the \#innotek channel or back
|
---|
| 1032 | * out the change.
|
---|
| 1033 | *
|
---|
[68445] | 1034 | * </ul>
|
---|
| 1035 | *
|
---|
[6303] | 1036 | * (Inspired by mozilla tree rules.)
|
---|
[72781] | 1037 | *
|
---|
| 1038 | *
|
---|
[6303] | 1039 | */
|
---|
| 1040 |
|
---|