VirtualBox

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

Last change on this file since 30720 was 28800, checked in by vboxsync, 15 years ago

Automated rebranding to Oracle copyright/license strings via filemuncher

  • Property svn:eol-style set to native
  • Property svn:keywords set to Author Date Id Revision
File size: 19.4 KB
Line 
1/* $Id: VBox-CodingGuidelines.cpp 28800 2010-04-27 08:22:32Z vboxsync $ */
2/** @file
3 * VBox - Coding Guidelines.
4 */
5
6/*
7 * Copyright (C) 2006-2009 Oracle Corporation
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
18/** @page pg_vbox_guideline VBox Coding Guidelines
19 *
20 * The VBox Coding guidelines are followed by all of VBox with the exception of
21 * qemu. Qemu is using whatever the frenchman does.
22 *
23 * There are a few compulsory rules and a bunch of optional ones. The following
24 * sections will describe these in details. In addition there is a section of
25 * Subversion 'rules'.
26 *
27 *
28 *
29 * @section sec_vbox_guideline_compulsory Compulsory
30 *
31 *
32 * - Use RT and VBOX types.
33 *
34 * - Use Runtime functions.
35 *
36 * - Use the standard bool, uintptr_t, intptr_t and [u]int[1-9+]_t types.
37 *
38 * - Avoid using plain unsigned and int.
39 *
40 * - Use static wherever possible. This makes the namespace less polluted
41 * and avoids nasty name clash problems which can occur, especially on
42 * Unix-like systems. (1)
43 *
44 * - Public names are of the form Domain[Subdomain[]]Method, using mixed
45 * casing to mark the words. The main domain is all uppercase.
46 * (Think like java, mapping domain and subdomain to packages/classes.)
47 *
48 * - Public names are always declared using the appropriate DECL macro. (2)
49 *
50 * - Internal names starts with a lowercased main domain.
51 *
52 * - Defines are all uppercase and separate words with underscore.
53 * This applies to enum values too.
54 *
55 * - Typedefs are all uppercase and contain no underscores to distinguish
56 * them from defines.
57 *
58 * - Pointer typedefs start with 'P'. If pointer to const then 'PC'.
59 *
60 * - Function typedefs start with 'FN'. If pointer to FN then 'PFN'.
61 *
62 * - All files are case sensitive.
63 *
64 * - Slashes are unix slashes ('/') runtime converts when necessary.
65 *
66 * - char strings are UTF-8.
67 *
68 * - All functions return VBox status codes. There are three general
69 * exceptions from this:
70 * -# Predicate functions. These are function which are boolean in
71 * nature and usage. They return bool. The function name will
72 * include 'Has', 'Is' or similar.
73 * -# Functions which by nature cannot possibly fail.
74 * These return void.
75 * -# "Get"-functions which return what they ask for.
76 * A get function becomes a "Query" function if there is any
77 * doubt about getting what is ask for.
78 *
79 * - VBox status codes have three subdivisions:
80 * -# Errors, which are VERR_ prefixed and negative.
81 * -# Warnings, which are VWRN_ prefixed and positive.
82 * -# Informational, which are VINF_ prefixed and positive.
83 *
84 * - Platform/OS operation are generalized and put in the IPRT.
85 *
86 * - Other useful constructs are also put in the IPRT.
87 *
88 * - The code shall not cause compiler warnings. Check this on ALL
89 * the platforms.
90 *
91 * - All files have file headers with $Id and a file tag which describes
92 * the file in a sentence or two.
93 * Note: Remember to enable keyword expansion when adding files to svn.
94 *
95 * - All public functions are fully documented in Doxygen style using the
96 * javadoc dialect (using the 'at' instead of the 'slash' as
97 * commandprefix.)
98 *
99 * - All structures in header files are described, including all their
100 * members.
101 *
102 * - All modules have a documentation 'page' in the main source file which
103 * describes the intent and actual implementation.
104 *
105 * - Code which is doing things that are not immediately comprehensible
106 * shall include explanatory comments.
107 *
108 * - Documentation and comments are kept up to date.
109 *
110 * - Headers in /include/VBox shall not contain any slash-slash C++
111 * comments, only ANSI C comments!
112 *
113 * - Comments on \#else indicates what begins while the comment on a
114 * \#endif indicates what ended.
115 *
116 *
117 * (1) It is common practice on Unix to have a single symbol namespace for an
118 * entire process. If one is careless symbols might be resolved in a
119 * different way that one expects, leading to weird problems.
120 *
121 * (2) This is common practice among most projects dealing with modules in
122 * shared libraries. The Windows / PE __declspect(import) and
123 * __declspect(export) constructs are the main reason for this.
124 * OTOH, we do perhaps have a bit too detailed graining of this in VMM...
125 *
126 *
127 * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit
128 *
129 * Here are some amendments which address 64-bit vs. 32-bit portability issues.
130 *
131 * Some facts first:
132 *
133 * - On 64-bit Windows the type long remains 32-bit. On nearly all other
134 * 64-bit platforms long is 64-bit.
135 *
136 * - On all 64-bit platforms we care about, int is 32-bit, short is 16 bit
137 * and char is 8-bit.
138 * (I don't know about any platforms yet where this isn't true.)
139 *
140 * - size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on
141 * 64-bit platforms. (These are 32-bit on 32-bit platforms.)
142 *
143 * - There is no inline assembly support in the 64-bit Microsoft compilers.
144 *
145 *
146 * Now for the guidelines:
147 *
148 * - Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a
149 * pointer to integer. Use uintptr_t or intptr_t. If you have to use
150 * NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR.
151 *
152 * - RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__,
153 * __WIN64__ and __WIN__ because they are all deprecated and scheduled
154 * for removal (if not removed already). Do not use the compiler
155 * defined _WIN32, _WIN64, or similar either. The bitness can be
156 * determined by testing ARCH_BITS.
157 * Example:
158 * @code
159 * #ifdef RT_OS_WINDOWS
160 * // call win32/64 api.
161 * #endif
162 * #ifdef RT_OS_WINDOWS
163 * # if ARCH_BITS == 64
164 * // call win64 api.
165 * # else // ARCH_BITS == 32
166 * // call win32 api.
167 * # endif // ARCH_BITS == 32
168 * #else // !RT_OS_WINDOWS
169 * // call posix api
170 * #endif // !RT_OS_WINDOWS
171 * @endcode
172 *
173 * - There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS
174 * mentioned above. Use these defines instead of any predefined
175 * compiler stuff or defines from system headers.
176 *
177 * - RT_ARCH_X86 is defined when compiling for the x86 the architecture.
178 * Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar
179 * for this purpose.
180 *
181 * - RT_ARCH_AMD64 is defined when compiling for the AMD64 architecture.
182 * Do not use __AMD64__, __amd64__ or __x64_86__.
183 *
184 * - Take care and use size_t when you have to, esp. when passing a pointer
185 * to a size_t as a parameter.
186 *
187 * - Be wary of type promotion to (signed) integer. For example the
188 * following will cause u8 to be promoted to int in the shift, and then
189 * sign extended in the assignment 64-bit:
190 * @code
191 * uint8_t u8 = 0xfe;
192 * uint64_t u64 = u8 << 24;
193 * // u64 == 0xfffffffffe000000
194 * @endcode
195 *
196 *
197 * @subsection sec_vbox_guideline_compulsory_cppmain C++ guidelines for Main
198 *
199 * Main is currently (2009) full of hard-to-maintain code that uses complicated
200 * templates. The new mid-term goal for Main is to have less custom templates
201 * instead of more for the following reasons:
202 *
203 * - Template code is harder to read and understand. Custom templates create
204 * territories which only the code writer understands.
205 *
206 * - Errors in using templates create terrible C++ compiler messages.
207 *
208 * - Template code is really hard to look at in a debugger.
209 *
210 * - Templates slow down the compiler a lot.
211 *
212 * In particular, the following bits should be considered deprecated and should
213 * NOT be used in new code:
214 *
215 * - everything in include/iprt/cpputils.h (auto_ref_ptr, exception_trap_base,
216 * char_auto_ptr and friends)
217 *
218 * Generally, in many cases, a simple class with a proper destructor can achieve
219 * the same effect as a 1,000-line template include file, and the code is
220 * much more accessible that way.
221 *
222 * Using standard STL templates like std::list, std::vector and std::map is OK.
223 * Exceptions are:
224 *
225 * - Guest Additions because we don't want to link against libstdc++ there.
226 *
227 * - std::string should not be used because we have iprt::MiniString and
228 * com::Utf8Str which can convert efficiently with COM's UTF-16 strings.
229 *
230 * - std::auto_ptr<> in general; that part of the C++ standard is just broken.
231 * Write a destructor that calls delete.
232 *
233 *
234 * @subsection sec_vbox_guideline_compulsory_cppqtgui C++ guidelines for the Qt GUI
235 *
236 * The Qt GUI is currently (2010) on its way to become more compatible to the
237 * rest of VirtualBox coding style wise. From now on, all the coding style
238 * rules described in this file are also mandatory for the Qt GUI. Additionally
239 * the following rules should be respected:
240 *
241 * - GUI classes which correspond to GUI tasks should be prefixed by UI (no VBox anymore)
242 *
243 * - Classes which extents some of the Qt classes should be prefix by QI
244 *
245 * - General task classes should be prefixed by C
246 *
247 * - Slots are prefixed by slt -> sltName
248 *
249 * - Signals are prefixed by sig -> sigName
250 *
251 * - Use Qt classes for lists, strings and so on, the use of STL classes should
252 * be avoided
253 *
254 * - All files like .cpp, .h, .ui, which belong together are located in the
255 * same directory and named the same
256 *
257 *
258 * @section sec_vbox_guideline_optional Optional
259 *
260 * First part is the actual coding style and all the prefixes. The second part
261 * is a bunch of good advice.
262 *
263 *
264 * @subsection sec_vbox_guideline_optional_layout The code layout
265 *
266 * - Curly brackets are not indented.
267 *
268 * - Space before the parenthesis when it comes after a C keyword.
269 *
270 * - No space between argument and parenthesis. Exception for complex
271 * expression.
272 * Example:
273 * @code
274 * if (PATMR3IsPatchGCAddr(pVM, GCPtr))
275 * @endcode
276 *
277 * - The else of an if is always the first statement on a line. (No curly
278 * stuff before it!)
279 *
280 * - else and if go on the same line if no { compound statement }
281 * follows the if.
282 * Example:
283 * @code
284 * if (fFlags & MYFLAGS_1)
285 * fFlags &= ~MYFLAGS_10;
286 * else if (fFlags & MYFLAGS_2)
287 * {
288 * fFlags &= ~MYFLAGS_MASK;
289 * fFlags |= MYFLAGS_5;
290 * }
291 * else if (fFlags & MYFLAGS_3)
292 * @endcode
293 *
294 * - The case is indented from the switch.
295 *
296 * - If a case needs curly brackets they contain the entire case, are not
297 * indented from the case, and the break or return is placed inside them.
298 * Example:
299 * @code
300 * switch (pCur->eType)
301 * {
302 * case PGMMAPPINGTYPE_PAGETABLES:
303 * {
304 * unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT;
305 * unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT;
306 * while (iPT-- > 0)
307 * if (pPD->a[iPDE + iPT].n.u1Present)
308 * return VERR_HYPERVISOR_CONFLICT;
309 * break;
310 * }
311 * }
312 * @endcode
313 *
314 * - In a do while construction, the while is on the same line as the
315 * closing "}" if any are used.
316 * Example:
317 * @code
318 * do
319 * {
320 * stuff;
321 * i--;
322 * } while (i > 0);
323 * @endcode
324 *
325 * - Comments are in C style. C++ style comments are used for temporary
326 * disabling a few lines of code.
327 *
328 * - Slightly complex boolean expressions are split into multiple lines,
329 * putting the operators first on the line and indenting it all according
330 * to the nesting of the expression. The purpose is to make it as easy as
331 * possible to read.
332 * Example:
333 * @code
334 * if ( RT_SUCCESS(rc)
335 * || (fFlags & SOME_FLAG))
336 * @endcode
337 *
338 * - No unnecessary parentheses in expressions (just don't over do this
339 * so that gcc / msc starts bitching). Find a correct C/C++ operator
340 * precedence table if needed.
341 *
342 *
343 * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes
344 *
345 * - The 'g_' (or 'g') prefix means a global variable, either on file or module level.
346 *
347 * - The 's_' (or 's') prefix means a static variable inside a function or class.
348 *
349 * - The 'm_' (or 'm') prefix means a class data member.
350 *
351 * In new code in Main, use "m_" (and common sense). As an exception, in Main,
352 * if a class encapsulates its member variables in an anonymous
353 * structure which is declared in the class, but defined only in the
354 * implementation (like this: "class X { struct Data; Data *m; }"), then
355 * the pointer to that struct is called "m" itself and its members then need no prefix,
356 * because the members are accessed with "m->member" already which is clear enough.
357 *
358 * - The 'p' prefix means pointer. For instance 'pVM' is pointer to VM.
359 *
360 * - The 'a' prefix means array. For instance 'aPages' could be read as array
361 * of pages.
362 *
363 * - The 'c' prefix means count. For instance 'cbBlock' could be read, count
364 * of bytes in block.
365 *
366 * - The 'off' prefix means offset.
367 *
368 * - The 'i' or 'idx' prefixes usually means index. Although the 'i' one can
369 * sometimes just mean signed integer.
370 *
371 * - The 'e' (or 'enm') prefix means enum.
372 *
373 * - The 'u' prefix usually means unsigned integer. Exceptions follows.
374 *
375 * - The 'u[1-9]+' prefix means a fixed bit size variable. Frequently used
376 * with the uint[1-9]+_t types and with bitfields.
377 *
378 * - The 'b' prefix means byte or bytes.
379 *
380 * - The 'f' prefix means flags. Flags are unsigned integers of some kind or bools.
381 *
382 * - The 'ch' prefix means a char, the (signed) char type.
383 *
384 * - The 'wc' prefix means a wide/windows char, the RTUTF16 type.
385 *
386 * - The 'uc' prefix means a Unicode Code point, the RTUNICP type.
387 *
388 * - The 'uch' prefix means unsigned char. It's rarely used.
389 *
390 * - The 'sz' prefix means zero terminated character string (array of chars). (UTF-8)
391 *
392 * - The 'wsz' prefix means zero terminated wide/windows character string (array of RTUTF16).
393 *
394 * - The 'usz' prefix means zero terminated Unicode string (array of RTUNICP).
395 *
396 * - The 'str' prefix means C++ string; either a std::string or, in Main,
397 * a Utf8Str or, in Qt, a QString
398 *
399 * - The 'bstr' prefix, in Main, means a UTF-16 Bstr.
400 *
401 * - The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback'
402 * and such like.
403 *
404 *
405 * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff
406 *
407 * - When writing code think as the reader.
408 *
409 * - When writing code think as the compiler. (2)
410 *
411 * - When reading code think as if it's full of bugs - find them and fix them.
412 *
413 * - Pointer within range tests like:
414 * @code
415 * if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange)
416 * @endcode
417 * Can also be written as (assuming cbRange unsigned):
418 * @code
419 * if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange)
420 * @endcode
421 * Which is shorter and potentially faster. (1)
422 *
423 * - Avoid unnecessary casting. All pointers automatically cast down to
424 * void *, at least for non class instance pointers.
425 *
426 * - It's very very bad practise to write a function larger than a
427 * screen full (1024x768) without any comprehensibility and explaining
428 * comments.
429 *
430 * - More to come....
431 *
432 *
433 * (1) Important, be very careful with the casting. In particular, note that
434 * a compiler might treat pointers as signed (IIRC).
435 *
436 * (2) "A really advanced hacker comes to understand the true inner workings of
437 * the machine - he sees through the language he's working in and glimpses
438 * the secret functioning of the binary code - becomes a Ba'al Shem of
439 * sorts." (Neal Stephenson "Snow Crash")
440 *
441 *
442 *
443 * @section sec_vbox_guideline_warnings Compiler Warnings
444 *
445 * The code should when possible compile on all platforms and compilers without any
446 * warnings. That's a nice idea, however, if it means making the code harder to read,
447 * less portable, unreliable or similar, the warning should not be fixed.
448 *
449 * Some of the warnings can seem kind of innocent at first glance. So, let's take the
450 * most common ones and explain them.
451 *
452 *
453 * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare
454 *
455 * GCC says: "warning: comparison between signed and unsigned integer expressions"
456 * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch"
457 *
458 * The following example will not output what you expect:
459@code
460#include <stdio.h>
461int main()
462{
463 signed long a = -1;
464 unsigned long b = 2294967295;
465 if (a < b)
466 printf("%ld < %lu: true\n", a, b);
467 else
468 printf("%ld < %lu: false\n", a, b);
469 return 0;
470}
471@endcode
472 * If I understood it correctly, the compiler will convert a to an
473 * unsigned long before doing the compare.
474 *
475 *
476 *
477 * @section sec_vbox_guideline_svn Subversion Commit Rules
478 *
479 *
480 * Before checking in:
481 *
482 * - Check Tinderbox and make sure the tree is green across all platforms. If it's
483 * red on a platform, don't check in. If you want, warn in the \#vbox channel and
484 * help make the responsible person fix it.
485 * NEVER CHECK IN TO A BROKEN BUILD.
486 *
487 * - When checking in keep in mind that a commit is atomic and that the Tinderbox and
488 * developers are constantly checking out the tree. Therefore do not split up the
489 * commit unless it's into 100% independent parts. If you need to split it up in order
490 * to have sensible commit comments, make the sub-commits as rapid as possible.
491 *
492 * - If you make a user visible change, such as fixing a reported bug,
493 * make sure you add an entry to doc/manual/user_ChangeLogImpl.xml.
494 *
495 * - If you are adding files make sure set the right attributes.
496 * svn-ps.sh/cmd was created for this purpose, please make use of it.
497 *
498 *
499 * After checking in:
500 *
501 * - After checking-in, you watch Tinderbox until your check-ins clear. You do not
502 * go home. You do not sleep. You do not log out or experiment with drugs. You do
503 * not become unavailable. If you break the tree, add a comment saying that you're
504 * fixing it. If you can't fix it and need help, ask in the \#innotek channel or back
505 * out the change.
506 *
507 * (Inspired by mozilla tree rules.)
508 */
509
Note: See TracBrowser for help on using the repository browser.

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