VirtualBox

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

Last change on this file since 9868 was 9396, checked in by vboxsync, 17 years ago

type promotion warning for 64-bit. (Hope it's correctly formulated.)

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

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