VirtualBox

source: vbox/trunk/doc/VBox-CodingGuidelines.cpp@ 21440

Last change on this file since 21440 was 21440, checked in by vboxsync, 16 years ago

Docs: some new C++ rules for Main and templates

  • Property svn:eol-style set to native
  • Property svn:keywords set to Author Date Id Revision
File size: 17.6 KB
Line 
1/* $Id: VBox-CodingGuidelines.cpp 21440 2009-07-09 13:42:43Z vboxsync $ */
2/** @file
3 * VBox - Coding Guidelines.
4 */
5
6/*
7 * Copyright (C) 2006-2009 Sun Microsystems, Inc.
8 *
9 * This file is part of VirtualBox Open Source Edition (OSE), as
10 * available from http://www.215389.xyz. 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 avoids nasty name clash problems which can occur, especially on
47 * Unix-like systems. (1)
48 *
49 * - Public names are of 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 then 'PC'.
64 *
65 * - Function typedefs start with 'FN'. If pointer to FN 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 return VBox status codes. There are three general
74 * exceptions from this:
75 * -# Predicate functions. These are function which are boolean in
76 * nature and usage. They return bool. The function name will
77 * include '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 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' instead of the 'slash' as
102 * commandprefix.)
103 *
104 * - All structures in header files are described, including all their
105 * members.
106 *
107 * - All modules have a documentation 'page' in the main source file which
108 * describes the intent and actual implementation.
109 *
110 * - Code which is doing things that are not immediately comprehensible
111 * shall include explanatory comments.
112 *
113 * - Documentation and comments are kept up to date.
114 *
115 * - Headers in /include/VBox shall not contain any slash-slash C++
116 * comments, only ANSI C comments!
117 *
118 *
119 * (1) It is common practice on Unix to have a single symbol namespace for an
120 * entire process. If one is careless symbols might be resolved in a
121 * different way that one expects, leading to weird problems.
122 *
123 * (2) This is common practice among most projects dealing with modules in
124 * shared libraries. The Windows / PE __declspect(import) and
125 * __declspect(export) constructs are the main reason for this.
126 * OTOH, we do perhaps have a bit too detailed graining of this in VMM...
127 *
128 *
129 *
130 * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit
131 *
132 * Here are some amendments which address 64-bit vs. 32-bit portability issues.
133 *
134 * Some facts first:
135 *
136 * - On 64-bit Windows the type long remains 32-bit. On nearly all other
137 * 64-bit platforms long is 64-bit.
138 *
139 * - On all 64-bit platforms we care about, int is 32-bit, short is 16 bit
140 * and char is 8-bit.
141 * (I don't know about any platforms yet where this isn't true.)
142 *
143 * - size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on
144 * 64-bit platforms. (These are 32-bit on 32-bit platforms.)
145 *
146 * - There is no inline assembly support in the 64-bit Microsoft compilers.
147 *
148 *
149 * Now for the guidelines:
150 *
151 * - Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a
152 * pointer to integer. Use uintptr_t or intptr_t. If you have to use
153 * NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR.
154 *
155 * - RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__,
156 * __WIN64__ and __WIN__ because they are all deprecated and scheduled
157 * for removal (if not removed already). Do not use the compiler
158 * defined _WIN32, _WIN64, or similar either. The bitness can be
159 * determined by testing ARCH_BITS.
160 * Example:
161 * @code
162 * #ifdef RT_OS_WINDOWS
163 * // call win32/64 api.
164 * #endif
165 * #ifdef RT_OS_WINDOWS
166 * # if ARCH_BITS == 64
167 * // call win64 api.
168 * # else // ARCH_BITS == 32
169 * // call win32 api.
170 * # endif // ARCH_BITS == 32
171 * #else // !RT_OS_WINDOWS
172 * // call posix api
173 * #endif // !RT_OS_WINDOWS
174 * @endcode
175 *
176 * - There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS
177 * mentioned above. Use these defines instead of any predefined
178 * compiler stuff or defines from system headers.
179 *
180 * - RT_ARCH_X86 is defined when compiling for the x86 the architecture.
181 * Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar
182 * for this purpose.
183 *
184 * - RT_ARCH_AMD64 is defined when compiling for the AMD64 architecture.
185 * Do not use __AMD64__, __amd64__ or __x64_86__.
186 *
187 * - Take care and use size_t when you have to, esp. when passing a pointer
188 * to a size_t as a parameter.
189 *
190 * - Be wary of type promotion to (signed) integer. For example the
191 * following will cause u8 to be promoted to int in the shift, and then
192 * sign extended in the assignment 64-bit:
193 * @code
194 * uint8_t u8 = 0xfe;
195 * uint64_t u64 = u8 << 24;
196 * // u64 == 0xfffffffffe000000
197 * @endcode
198 *
199 *
200 *
201 * @subsection sec_vbox_guideline_compulsory_cppmain C++ guidelines for Main
202 *
203 * Main is currently (2009) full of hard-to-maintain code that uses complicated
204 * templates. The new mid-term goal for Main is to have less custom templates
205 * instead of more for the following reasons:
206 *
207 * - Template code is harder to read and understand. Custom templates create
208 * territories which only the code writer understands.
209 *
210 * - Errors in using templates create terrible C++ compiler messages.
211 *
212 * - Template code is really hard to look at in a debugger.
213 *
214 * - Templates slow down the compiler a lot.
215 *
216 * In particular, the following bits should be considered deprecated and should
217 * NOT be used in new code:
218 *
219 * - everything in include/iprt/cpputils.h (auto_ref_ptr, exception_trap_base,
220 * char_auto_ptr and friends)
221 *
222 * Generally, in many cases, a simple class with a proper destructor can achieve
223 * the same effect as a 1,000-line template include file, and the code is
224 * much more accessible that way.
225 *
226 * Using standard STL templates like std::list, std::vector and std::map is OK.
227 * Exceptions are:
228 *
229 * - Guest Additions because we don't want to link against libstdc++ there.
230 *
231 * - std::string should not be used because we have iprt::MiniString and
232 * com::Utf8Str which can convert efficiently with COM's UTF-16 strings.
233 *
234 * - std::auto_ptr<> in general; that part of the C++ standard is just broken.
235 * Write a destructor that calls delete.
236 *
237 *
238 * @section sec_vbox_guideline_optional Optional
239 *
240 * First part is the actual coding style and all the prefixes. The second part
241 * is a bunch of good advice.
242 *
243 *
244 * @subsection sec_vbox_guideline_optional_layout The code layout
245 *
246 * - Curly brackets are not indented.
247 *
248 * - Space before the parenthesis when it comes after a C keyword.
249 *
250 * - No space between argument and parenthesis. Exception for complex
251 * expression.
252 * Example:
253 * @code
254 * if (PATMR3IsPatchGCAddr(pVM, GCPtr))
255 * @endcode
256 *
257 * - The else of an if is always the first statement on a line. (No curly
258 * stuff before it!)
259 *
260 * - else and if go on the same line if no { compound statement }
261 * follows the if.
262 * Example:
263 * @code
264 * if (fFlags & MYFLAGS_1)
265 * fFlags &= ~MYFLAGS_10;
266 * else if (fFlags & MYFLAGS_2)
267 * {
268 * fFlags &= ~MYFLAGS_MASK;
269 * fFlags |= MYFLAGS_5;
270 * }
271 * else if (fFlags & MYFLAGS_3)
272 * @endcode
273 *
274 * - The case is indented from the switch.
275 *
276 * - If a case needs curly brackets they contain the entire case, are not
277 * indented from the case, and the break or return is placed inside them.
278 * Example:
279 * @code
280 * switch (pCur->eType)
281 * {
282 * case PGMMAPPINGTYPE_PAGETABLES:
283 * {
284 * unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT;
285 * unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT;
286 * while (iPT-- > 0)
287 * if (pPD->a[iPDE + iPT].n.u1Present)
288 * return VERR_HYPERVISOR_CONFLICT;
289 * break;
290 * }
291 * }
292 * @endcode
293 *
294 * - In a do while construction, the while is on the same line as the
295 * closing "}" if any are used.
296 * Example:
297 * @code
298 * do
299 * {
300 * stuff;
301 * i--;
302 * } while (i > 0);
303 * @endcode
304 *
305 * - Comments are in C style. C++ style comments are used for temporary
306 * disabling a few lines of code.
307 *
308 * - Slightly complex boolean expressions are split into multiple lines,
309 * putting the operators first on the line and indenting it all according
310 * to the nesting of the expression. The purpose is to make it as easy as
311 * possible to read.
312 * Example:
313 * @code
314 * if ( RT_SUCCESS(rc)
315 * || (fFlags & SOME_FLAG))
316 * @endcode
317 *
318 * - No unnecessary parentheses in expressions (just don't over do this
319 * so that gcc / msc starts bitching). Find a correct C/C++ operator
320 * precedence table if needed.
321 *
322 *
323 * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes
324 *
325 * - The 'g_' (or 'g') prefix means a global variable, either on file or module level.
326 *
327 * - The 's_' (or 's') prefix means a static variable inside a function or class.
328 *
329 * - The 'm_' (or 'm') prefix means a class data member.
330 *
331 * - The 'p' prefix means pointer. For instance 'pVM' is pointer to VM.
332 *
333 * - The 'a' prefix means array. For instance 'aPages' could be read as array
334 * of pages.
335 *
336 * - The 'c' prefix means count. For instance 'cbBlock' could be read, count
337 * of bytes in block.
338 *
339 * - The 'off' prefix means offset.
340 *
341 * - The 'i' or 'idx' prefixes usually means index. Although the 'i' one can
342 * sometimes just mean signed integer.
343 *
344 * - The 'e' (or 'enm') prefix means enum.
345 *
346 * - The 'u' prefix usually means unsigned integer. Exceptions follows.
347 *
348 * - The 'u[1-9]+' prefix means a fixed bit size variable. Frequently used
349 * with the uint[1-9]+_t types and with bitfields.
350 *
351 * - The 'b' prefix means byte or bytes.
352 *
353 * - The 'f' prefix means flags. Flags are unsigned integers of some kind or bools.
354 *
355 * - The 'ch' prefix means a char, the (signed) char type.
356 *
357 * - The 'wc' prefix means a wide/windows char, the RTUTF16 type.
358 *
359 * - The 'uc' prefix means a Unicode Code point, the RTUNICP type.
360 *
361 * - The 'uch' prefix means unsigned char. It's rarely used.
362 *
363 * - The 'sz' prefix means zero terminated character string (array of chars). (UTF-8)
364 *
365 * - The 'wsz' prefix means zero terminated wide/windows character string (array of RTUTF16).
366 *
367 * - The 'usz' prefix means zero terminated Unicode string (array of RTUNICP).
368 *
369 * - The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback'
370 * and such like.
371 *
372 *
373 * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff
374 *
375 * - When writing code think as the reader.
376 *
377 * - When writing code think as the compiler.
378 *
379 * - When reading code think as if it's full of bugs - find them and fix them.
380 *
381 * - Pointer within range tests like:
382 * @code
383 * if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange)
384 * @endcode
385 * Can also be written as (assuming cbRange unsigned):
386 * @code
387 * if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange)
388 * @endcode
389 * Which is shorter and potentially faster. (1)
390 *
391 * - Avoid unnecessary casting. All pointers automatically cast down to
392 * void *, at least for non class instance pointers.
393 *
394 * - It's very very bad practise to write a function larger than a
395 * screen full (1024x768) without any comprehensibility and explaining
396 * comments.
397 *
398 * - More to come....
399 *
400 *
401 * (1) Important, be very careful with the casting. In particular, note that
402 * a compiler might treat pointers as signed (IIRC).
403 *
404 *
405 *
406 *
407 * @section sec_vbox_guideline_warnings Compiler Warnings
408 *
409 * The code should when possible compile on all platforms and compilers without any
410 * warnings. That's a nice idea, however, if it means making the code harder to read,
411 * less portable, unreliable or similar, the warning should not be fixed.
412 *
413 * Some of the warnings can seem kind of innocent at first glance. So, let's take the
414 * most common ones and explain them.
415 *
416 * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare
417 *
418 * GCC says: "warning: comparison between signed and unsigned integer expressions"
419 * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch"
420 *
421 * The following example will not output what you expect:
422@code
423#include <stdio.h>
424int main()
425{
426 signed long a = -1;
427 unsigned long b = 2294967295;
428 if (a < b)
429 printf("%ld < %lu: true\n", a, b);
430 else
431 printf("%ld < %lu: false\n", a, b);
432 return 0;
433}
434@endcode
435 * If I understood it correctly, the compiler will convert a to an
436 * unsigned long before doing the compare.
437 *
438 *
439 * @section sec_vbox_guideline_svn Subversion Commit Rules
440 *
441 *
442 * Before checking in:
443 *
444 * - Check Tinderbox and make sure the tree is green across all platforms. If it's
445 * red on a platform, don't check in. If you want, warn in the \#vbox channel and
446 * help make the responsible person fix it.
447 * NEVER CHECK IN TO A BROKEN BUILD.
448 *
449 * - When checking in keep in mind that a commit is atomic and that the Tinderbox and
450 * developers are constantly checking out the tree. Therefore do not split up the
451 * commit unless it's into 100% independent parts. If you need to split it up in order
452 * to have sensible commit comments, make the sub-commits as rapid as possible.
453 *
454 * - If you make a user visible change, such as fixing a reported bug,
455 * make sure you add an entry to doc/manual/user_ChangeLogImpl.xml.
456 *
457 *
458 * After checking in:
459 *
460 * - After checking-in, you watch Tinderbox until your check-ins clear. You do not
461 * go home. You do not sleep. You do not log out or experiment with drugs. You do
462 * not become unavailable. If you break the tree, add a comment saying that you're
463 * fixing it. If you can't fix it and need help, ask in the \#innotek channel or back
464 * out the change.
465 *
466 * (Inspired by mozilla tree rules.)
467 */
468
Note: See TracBrowser for help on using the repository browser.

© 2025 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette