VirtualBox

root/trunk/doc/VBox-CodingGuidelines.cpp

Revision 9396, 16.0 kB (checked in by vboxsync, 3 months 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
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>
384 int 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 browser.

© 2008 Sun Microsystems, Inc.
ContactPrivacy policy