VirtualBox

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

Last change on this file since 31670 was 30954, checked in by vboxsync, 14 years ago

doc/VBox-CodingGuidelines.cpp: Added bits that was asked for on the mailing list as well as documenting some prefixes we're using.

  • Property svn:eol-style set to native
  • Property svn:keywords set to Author Date Id Revision
File size: 23.7 KB
Line 
1/* $Id: VBox-CodingGuidelines.cpp 30954 2010-07-21 12:37:49Z 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 * - The indentation size is 4 chars.
33 *
34 * - Tabs are only ever used in makefiles.
35 *
36 * - Use RT and VBOX types.
37 *
38 * - Use Runtime functions.
39 *
40 * - Use the standard bool, uintptr_t, intptr_t and [u]int[1-9+]_t types.
41 *
42 * - Avoid using plain unsigned and int.
43 *
44 * - Use static wherever possible. This makes the namespace less polluted
45 * and avoids nasty name clash problems which can occur, especially on
46 * Unix-like systems. (1)
47 *
48 * - Public names are of the form Domain[Subdomain[]]Method, using mixed
49 * casing to mark the words. The main domain is all uppercase.
50 * (Think like java, mapping domain and subdomain to packages/classes.)
51 *
52 * - Public names are always declared using the appropriate DECL macro. (2)
53 *
54 * - Internal names starts with a lowercased main domain.
55 *
56 * - Defines are all uppercase and separate words with underscore.
57 * This applies to enum values too.
58 *
59 * - Typedefs are all uppercase and contain no underscores to distinguish
60 * them from defines.
61 *
62 * - Pointer typedefs start with 'P'. If pointer to const then 'PC'.
63 *
64 * - Function typedefs start with 'FN'. If pointer to FN then 'PFN'.
65 *
66 * - All files are case sensitive.
67 *
68 * - Slashes are unix slashes ('/') runtime converts when necessary.
69 *
70 * - char strings are UTF-8.
71 *
72 * - All functions return VBox status codes. There are three general
73 * exceptions from this:
74 * -# Predicate functions. These are function which are boolean in
75 * nature and usage. They return bool. The function name will
76 * include 'Has', 'Is' or similar.
77 * -# Functions which by nature cannot possibly fail.
78 * These return void.
79 * -# "Get"-functions which return what they ask for.
80 * A get function becomes a "Query" function if there is any
81 * doubt about getting what is ask for.
82 *
83 * - VBox status codes have three subdivisions:
84 * -# Errors, which are VERR_ prefixed and negative.
85 * -# Warnings, which are VWRN_ prefixed and positive.
86 * -# Informational, which are VINF_ prefixed and positive.
87 *
88 * - Platform/OS operation are generalized and put in the IPRT.
89 *
90 * - Other useful constructs are also put in the IPRT.
91 *
92 * - The code shall not cause compiler warnings. Check this on ALL
93 * the platforms.
94 *
95 * - All files have file headers with $Id and a file tag which describes
96 * the file in a sentence or two.
97 * Note: Remember to enable keyword expansion when adding files to svn.
98 *
99 * - All public functions are fully documented in Doxygen style using the
100 * javadoc dialect (using the 'at' instead of the 'slash' as
101 * commandprefix.)
102 *
103 * - All structures in header files are described, including all 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 immediately comprehensible
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++
115 * comments, only ANSI C comments!
116 *
117 * - Comments on \#else indicates what begins while the comment on a
118 * \#endif indicates what ended.
119 *
120 *
121 * (1) It is common practice on Unix to have a single symbol namespace for an
122 * entire process. If one is careless symbols might be resolved in a
123 * different way that one expects, leading to weird problems.
124 *
125 * (2) This is common practice among most projects dealing with modules in
126 * shared libraries. The Windows / PE __declspect(import) and
127 * __declspect(export) constructs are the main reason for this.
128 * OTOH, we do perhaps have a bit too detailed graining of this in VMM...
129 *
130 *
131 * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit
132 *
133 * Here are some amendments which address 64-bit vs. 32-bit portability issues.
134 *
135 * Some facts first:
136 *
137 * - On 64-bit Windows the type long remains 32-bit. On nearly all other
138 * 64-bit platforms long is 64-bit.
139 *
140 * - On all 64-bit platforms we care about, int is 32-bit, short is 16 bit
141 * and char is 8-bit.
142 * (I don't know about any platforms yet where this isn't true.)
143 *
144 * - size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on
145 * 64-bit platforms. (These are 32-bit on 32-bit platforms.)
146 *
147 * - There is no inline assembly support in the 64-bit Microsoft compilers.
148 *
149 *
150 * Now for the guidelines:
151 *
152 * - Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a
153 * pointer to integer. Use uintptr_t or intptr_t. If you have to use
154 * NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR.
155 *
156 * - RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__,
157 * __WIN64__ and __WIN__ because they are all deprecated and scheduled
158 * for removal (if not removed already). Do not use the compiler
159 * defined _WIN32, _WIN64, or similar either. The bitness can be
160 * determined by testing ARCH_BITS.
161 * Example:
162 * @code
163 * #ifdef RT_OS_WINDOWS
164 * // call win32/64 api.
165 * #endif
166 * #ifdef RT_OS_WINDOWS
167 * # if ARCH_BITS == 64
168 * // call win64 api.
169 * # else // ARCH_BITS == 32
170 * // call win32 api.
171 * # endif // ARCH_BITS == 32
172 * #else // !RT_OS_WINDOWS
173 * // call posix api
174 * #endif // !RT_OS_WINDOWS
175 * @endcode
176 *
177 * - There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS
178 * mentioned above. Use these defines instead of any predefined
179 * compiler stuff or defines from system headers.
180 *
181 * - RT_ARCH_X86 is defined when compiling for the x86 the architecture.
182 * Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar
183 * for this purpose.
184 *
185 * - RT_ARCH_AMD64 is defined when compiling for the AMD64 architecture.
186 * Do not use __AMD64__, __amd64__ or __x64_86__.
187 *
188 * - Take care and use size_t when you have to, esp. when passing a pointer
189 * to a size_t as a parameter.
190 *
191 * - Be wary of type promotion to (signed) integer. For example the
192 * following will cause u8 to be promoted to int in the shift, and then
193 * sign extended in the assignment 64-bit:
194 * @code
195 * uint8_t u8 = 0xfe;
196 * uint64_t u64 = u8 << 24;
197 * // u64 == 0xfffffffffe000000
198 * @endcode
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 * @subsection sec_vbox_guideline_compulsory_cppqtgui C++ guidelines for the Qt GUI
239 *
240 * The Qt GUI is currently (2010) on its way to become more compatible to the
241 * rest of VirtualBox coding style wise. From now on, all the coding style
242 * rules described in this file are also mandatory for the Qt GUI. Additionally
243 * the following rules should be respected:
244 *
245 * - GUI classes which correspond to GUI tasks should be prefixed by UI (no VBox anymore)
246 *
247 * - Classes which extents some of the Qt classes should be prefix by QI
248 *
249 * - General task classes should be prefixed by C
250 *
251 * - Slots are prefixed by slt -> sltName
252 *
253 * - Signals are prefixed by sig -> sigName
254 *
255 * - Use Qt classes for lists, strings and so on, the use of STL classes should
256 * be avoided
257 *
258 * - All files like .cpp, .h, .ui, which belong together are located in the
259 * same directory and named the same
260 *
261 *
262 * @section sec_vbox_guideline_optional Optional
263 *
264 * First part is the actual coding style and all the prefixes. The second part
265 * is a bunch of good advice.
266 *
267 *
268 * @subsection sec_vbox_guideline_optional_layout The code layout
269 *
270 * - Max line length is 130 chars. Exceptions are table-like
271 * code/initializers and Log*() statements (don't waste unnecessary
272 * vertical space on debug logging).
273 *
274 * - Comments should try stay within the usual 80 columns.
275 *
276 * - Curly brackets are not indented. Example:
277 * @code
278 * if (true)
279 * {
280 * Something1();
281 * Something2();
282 * }
283 * else
284 * {
285 * SomethingElse1().
286 * SomethingElse2().
287 * }
288 * @endcode
289 *
290 * - Space before the parentheses when it comes after a C keyword.
291 *
292 * - No space between argument and parentheses. Exception for complex
293 * expression. Example:
294 * @code
295 * if (PATMR3IsPatchGCAddr(pVM, GCPtr))
296 * @endcode
297 *
298 * - The else of an if is always the first statement on a line. (No curly
299 * stuff before it!)
300 *
301 * - else and if go on the same line if no { compound statement }
302 * follows the if. Example:
303 * @code
304 * if (fFlags & MYFLAGS_1)
305 * fFlags &= ~MYFLAGS_10;
306 * else if (fFlags & MYFLAGS_2)
307 * {
308 * fFlags &= ~MYFLAGS_MASK;
309 * fFlags |= MYFLAGS_5;
310 * }
311 * else if (fFlags & MYFLAGS_3)
312 * @endcode
313 *
314 *
315 * - Slightly complex boolean expressions are split into multiple lines,
316 * putting the operators first on the line and indenting it all according
317 * to the nesting of the expression. The purpose is to make it as easy as
318 * possible to read. Example:
319 * @code
320 * if ( RT_SUCCESS(rc)
321 * || (fFlags & SOME_FLAG))
322 * @endcode
323 *
324 * - When 'if' or 'while' statements gets long, the closing parentheses
325 * goes right below the opening parentheses. This may be applied to
326 * sub-expression. Example:
327 * @code
328 * if ( RT_SUCCESS(rc)
329 * || ( fSomeStuff
330 * && fSomeOtherStuff
331 * && fEvenMoreStuff
332 * )
333 * || SomePredicateFunction()
334 * )
335 * {
336 * ...
337 * }
338 * @endcode
339 *
340 * - The case is indented from the switch (to avoid having the braces for
341 * the 'case' at the same level as the 'switch' statement).
342 *
343 * - If a case needs curly brackets they contain the entire case, are not
344 * indented from the case, and the break or return is placed inside them.
345 * Example:
346 * @code
347 * switch (pCur->eType)
348 * {
349 * case PGMMAPPINGTYPE_PAGETABLES:
350 * {
351 * unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT;
352 * unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT;
353 * while (iPT-- > 0)
354 * if (pPD->a[iPDE + iPT].n.u1Present)
355 * return VERR_HYPERVISOR_CONFLICT;
356 * break;
357 * }
358 * }
359 * @endcode
360 *
361 * - In a do while construction, the while is on the same line as the
362 * closing "}" if any are used.
363 * Example:
364 * @code
365 * do
366 * {
367 * stuff;
368 * i--;
369 * } while (i > 0);
370 * @endcode
371 *
372 * - Comments are in C style. C++ style comments are used for temporary
373 * disabling a few lines of code.
374 *
375 * - No unnecessary parentheses in expressions (just don't over do this
376 * so that gcc / msc starts bitching). Find a correct C/C++ operator
377 * precedence table if needed.
378 *
379 * - 'for (;;)' is preferred over 'while (true)' and 'while (1)'.
380 *
381 * - Parameters are indented to the start parentheses when breaking up
382 * function calls, declarations or prototypes. (This is in line with
383 * how 'if', 'for' and 'while' statements are done as well.) Example:
384 * @code
385 * RTPROCESS hProcess;
386 * int rc = RTProcCreateEx(papszArgs[0],
387 * papszArgs,
388 * RTENV_DEFAULT,
389 * fFlags,
390 * NULL, // phStdIn
391 * NULL, // phStdOut
392 * NULL, // phStdErr
393 * NULL, // pszAsUser
394 * NULL, // pszPassword
395 * &hProcess);
396 * @endcode
397 *
398 * - That Dijkstra is dead is no excuse for using gotos.
399 *
400 *
401 *
402 * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes
403 *
404 * - The 'g_' (or 'g') prefix means a global variable, either on file or module level.
405 *
406 * - The 's_' (or 's') prefix means a static variable inside a function or class.
407 *
408 * - The 'm_' (or 'm') prefix means a class data member.
409 *
410 * In new code in Main, use "m_" (and common sense). As an exception,
411 * in Main, if a class encapsulates its member variables in an anonymous
412 * structure which is declared in the class, but defined only in the
413 * implementation (like this: 'class X { struct Data; Data *m; }'), then
414 * the pointer to that struct is called 'm' itself and its members then
415 * need no prefix, because the members are accessed with 'm->member'
416 * already which is clear enough.
417 *
418 * - The 'a_' prefix means a parameter (argument) variable. This is
419 * sometimes written 'a' in parts of the source code that does not use
420 * the array prefix.
421 *
422 * - The 'p' prefix means pointer. For instance 'pVM' is pointer to VM.
423 *
424 * - The 'r' prefix means that something is passed by reference.
425 *
426 * - The 'k' prefix means that something is a constant. For instance
427 * 'enum { kStuff };'. This is usually not used in combination with
428 * 'p', 'r' or any such thing, it's main main use is to make enums
429 * easily identifiable.
430 *
431 * - The 'a' prefix means array. For instance 'aPages' could be read as
432 * array of pages.
433 *
434 * - The 'c' prefix means count. For instance 'cbBlock' could be read,
435 * count of bytes in block.
436 *
437 * - The 'off' prefix means offset.
438 *
439 * - The 'i' or 'idx' prefixes usually means index. Although the 'i' one
440 * can sometimes just mean signed integer.
441 *
442 * - The 'i[1-9]+' prefix means a fixed bit size variable. Frequently
443 * used with the int[1-9]+_t types. [type]
444 *
445 * - The 'e' (or 'enm') prefix means enum.
446 *
447 * - The 'u' prefix usually means unsigned integer. Exceptions follows.
448 *
449 * - The 'u[1-9]+' prefix means a fixed bit size variable. Frequently
450 * used with the uint[1-9]+_t types and with bitfields. [type]
451 *
452 * - The 'b' prefix means byte or bytes. [type]
453 *
454 * - The 'f' prefix means flags. Flags are unsigned integers of some kind
455 * or booleans.
456 *
457 * - TODO: need prefix for real float. [type]
458 *
459 * - The 'rd' prefix means real double and is used for 'double' variables.
460 * [type]
461 *
462 * - The 'lrd' prefix means long real double and is used for 'long double'
463 * variables. [type]
464 *
465 * - The 'ch' prefix means a char, the (signed) char type. [type]
466 *
467 * - The 'wc' prefix means a wide/windows char, the RTUTF16 type. [type]
468 *
469 * - The 'uc' prefix means a Unicode Code point, the RTUNICP type. [type]
470 *
471 * - The 'uch' prefix means unsigned char. It's rarely used. [type]
472 *
473 * - The 'sz' prefix means zero terminated character string (array of
474 * chars). (UTF-8)
475 *
476 * - The 'wsz' prefix means zero terminated wide/windows character string
477 * (array of RTUTF16).
478 *
479 * - The 'usz' prefix means zero terminated Unicode string (array of
480 * RTUNICP).
481 *
482 * - The 'str' prefix means C++ string; either a std::string or, in Main,
483 * a Utf8Str or, in Qt, a QString. When used with 'p', 'r', 'a' or 'c'
484 * the first letter should be capitalized.
485 *
486 * - The 'bstr' prefix, in Main, means a UTF-16 Bstr. When used with 'p',
487 * 'r', 'a' or 'c' the first letter should be capitalized.
488 *
489 * - The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback'
490 * and such like.
491 *
492 * - The 'psz' prefix is a combination of 'p' and 'sz' and thus means
493 * pointer to a zero terminated character string. (UTF-8)
494 *
495 * - The 'pcsz' prefix is used to indicate constant string pointers in
496 * parts of the code. Most code uses 'psz' for const and non-const
497 * string pointers.
498 *
499 * - The 'l' prefix means (signed) long. We try avoid using this,
500 * expecially with the 'LONG' types in Main as these are not 'long' on
501 * 64-bit non-Windows platforms and can cause confusion. Alternatives:
502 * 'i' or 'i32'. [type]
503 *
504 * - The 'ul' prefix means unsigned long. We try avoid using this,
505 * expecially with the 'ULONG' types in Main as these are not 'unsigned
506 * long' on 64-bit non-Windows platforms and can cause confusion.
507 * Alternatives: 'u' or 'u32'. [type]
508 *
509 *
510 * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff
511 *
512 * - When writing code think as the reader.
513 *
514 * - When writing code think as the compiler. (2)
515 *
516 * - When reading code think as if it's full of bugs - find them and fix them.
517 *
518 * - Pointer within range tests like:
519 * @code
520 * if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange)
521 * @endcode
522 * Can also be written as (assuming cbRange unsigned):
523 * @code
524 * if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange)
525 * @endcode
526 * Which is shorter and potentially faster. (1)
527 *
528 * - Avoid unnecessary casting. All pointers automatically cast down to
529 * void *, at least for non class instance pointers.
530 *
531 * - It's very very bad practise to write a function larger than a
532 * screen full (1024x768) without any comprehensibility and explaining
533 * comments.
534 *
535 * - More to come....
536 *
537 *
538 * (1) Important, be very careful with the casting. In particular, note that
539 * a compiler might treat pointers as signed (IIRC).
540 *
541 * (2) "A really advanced hacker comes to understand the true inner workings of
542 * the machine - he sees through the language he's working in and glimpses
543 * the secret functioning of the binary code - becomes a Ba'al Shem of
544 * sorts." (Neal Stephenson "Snow Crash")
545 *
546 *
547 *
548 * @section sec_vbox_guideline_warnings Compiler Warnings
549 *
550 * The code should when possible compile on all platforms and compilers without any
551 * warnings. That's a nice idea, however, if it means making the code harder to read,
552 * less portable, unreliable or similar, the warning should not be fixed.
553 *
554 * Some of the warnings can seem kind of innocent at first glance. So, let's take the
555 * most common ones and explain them.
556 *
557 *
558 * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare
559 *
560 * GCC says: "warning: comparison between signed and unsigned integer expressions"
561 * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch"
562 *
563 * The following example will not output what you expect:
564@code
565#include <stdio.h>
566int main()
567{
568 signed long a = -1;
569 unsigned long b = 2294967295;
570 if (a < b)
571 printf("%ld < %lu: true\n", a, b);
572 else
573 printf("%ld < %lu: false\n", a, b);
574 return 0;
575}
576@endcode
577 * If I understood it correctly, the compiler will convert a to an
578 * unsigned long before doing the compare.
579 *
580 *
581 *
582 * @section sec_vbox_guideline_svn Subversion Commit Rules
583 *
584 *
585 * Before checking in:
586 *
587 * - Check Tinderbox and make sure the tree is green across all platforms. If it's
588 * red on a platform, don't check in. If you want, warn in the \#vbox channel and
589 * help make the responsible person fix it.
590 * NEVER CHECK IN TO A BROKEN BUILD.
591 *
592 * - When checking in keep in mind that a commit is atomic and that the Tinderbox and
593 * developers are constantly checking out the tree. Therefore do not split up the
594 * commit unless it's into 100% independent parts. If you need to split it up in order
595 * to have sensible commit comments, make the sub-commits as rapid as possible.
596 *
597 * - If you make a user visible change, such as fixing a reported bug,
598 * make sure you add an entry to doc/manual/user_ChangeLogImpl.xml.
599 *
600 * - If you are adding files make sure set the right attributes.
601 * svn-ps.sh/cmd was created for this purpose, please make use of it.
602 *
603 *
604 * After checking in:
605 *
606 * - After checking-in, you watch Tinderbox until your check-ins clear. You do not
607 * go home. You do not sleep. You do not log out or experiment with drugs. You do
608 * not become unavailable. If you break the tree, add a comment saying that you're
609 * fixing it. If you can't fix it and need help, ask in the \#innotek channel or back
610 * out the change.
611 *
612 * (Inspired by mozilla tree rules.)
613 */
614
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