VBox-CodingGuidelines.cpp@ 72791

Last change on this file since 72791 was 72791, checked in by vboxsync, 7 years ago
doc/VBox-CodingGuidelines.cpp: Rewrote intro and added a table of contents. Put sec_vbox_guideline_local after toc.
Property svn:eol-style set to `native` Property svn:keywords set to `Author Date Id Revision`
File size: 36.6 KB

Line
1	/* $Id: VBox-CodingGuidelines.cpp 72791 2018-07-02 10:33:24Z vboxsync $ */
2	/** @file
3	* VBox - Coding Guidelines.
4	*/
5
6	/*
7	* Copyright (C) 2006-2017 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 compulsory sections of these guidelines are to be followed in all of the
21	* VBox sources. Please note that local guidelines in parts of the VBox source
22	* tree may promote the optional ones to compulsory status. The VBox tree also
23	* contains some 3rd party sources where it is good to follow the local coding
24	* style while keeping these guidelines in mind.
25	*
26	* Contents:
27	* - @ref sec_vbox_guideline_compulsory
28	* - @ref sec_vbox_guideline_compulsory_sub64
29	* - @ref sec_vbox_guideline_compulsory_cppmain
30	* - @ref sec_vbox_guideline_compulsory_cppqtgui
31	* - @ref sec_vbox_guideline_compulsory_xslt
32	* - @ref sec_vbox_guideline_compulsory_doxygen
33	* - @ref sec_vbox_guideline_compulsory_guest
34	* - @ref sec_vbox_guideline_optional
35	* - @ref sec_vbox_guideline_optional_layout
36	* - @ref sec_vbox_guideline_optional_prefix
37	* - @ref sec_vbox_guideline_optional_misc
38	* - @ref sec_vbox_guideline_warnings
39	* - @ref sec_vbox_guideline_warnings_signed_unsigned_compare
40	* - @ref sec_vbox_guideline_svn
41	*
42	* Local guidelines overrides:
43	* - src/VBox/VMM/: @ref pg_vmm_guideline (src/VBox/VMM/Docs-CodingGuidelines.cpp)
44	* - src/VBox/ValidationKit/: @ref pg_validationkit_guideline (src/VBox/ValidationKit/ValidationKitCodingGuidelines.cpp)
45	* - src/VBox/Runtime/: All of @ref sec_vbox_guideline_optional is mandatory.
46	* - src/VBox/Main/: @ref sec_vbox_guideline_compulsory_cppmain
47	* - src/VBox/Frontends/VirtualBox/: @ref sec_vbox_guideline_compulsory_cppqtgui
48	*
49	*
50	* @section sec_vbox_guideline_local Local Coding Guidelines
51	*
52	* VirtualBox is a large project, so there are parts with additional guidelines
53	* that extends the above ones.
54	*
55	* <ul>
56	*
57	* <li> src/VBox/VMM/: @ref pg_vmm_guideline
58	* (src/VBox/VMM/Docs-CodingGuidelines.cpp)
59	*
60	* <li> src/VBox/ValidationKit/: @ref pg_validationkit_guideline
61	* (src/VBox/ValidationKit/ValidationKitCodingGuidelines.cpp)
62	*
63	* <li> src/VBox/Runtime/: @ref sec_vbox_guideline_optional is mandatory.
64	*
65	* <li> src/VBox/Frontends/VirtualBox/: @ref sec_vbox_guideline_compulsory_cppqtgui
66	*
67	* <li> src/VBox/Main: @ref sec_vbox_guideline_compulsory_cppmain
68	*
69	*
70	* </ul>
71	*
72	*
73	* @section sec_vbox_guideline_compulsory Compulsory
74	*
75	* <ul>
76	*
77	* <li> The indentation size is 4 chars.
78	*
79	* <li> Tabs are only ever used in makefiles.
80	*
81	* <li> Use RT and VBOX types.
82	*
83	* <li> Use Runtime functions.
84	*
85	* <li> Use the standard bool, uintptr_t, intptr_t and [u]int[1-9+]_t types.
86	*
87	* <li> Avoid using plain unsigned and int.
88	*
89	* <li> Use static wherever possible. This makes the namespace less polluted
90	* and avoids nasty name clash problems which can occur, especially on
91	* Unix-like systems. (1) It also simplifies locating callers when
92	* changing it (single source file vs entire VBox tree).
93	*
94	* <li> Public names are of the form Domain[Subdomain[]]Method, using mixed
95	* casing to mark the words. The main domain is all uppercase.
96	* (Think like java, mapping domain and subdomain to packages/classes.)
97	*
98	* <li> Public names are always declared using the appropriate DECL macro. (2)
99	*
100	* <li> Internal names starts with a lowercased main domain.
101	*
102	* <li> Defines are all uppercase and separate words with underscore.
103	* This applies to enum values too.
104	*
105	* <li> Typedefs are all uppercase and contain no underscores to distinguish
106	* them from defines.
107	*
108	* <li> Pointer typedefs start with 'P'. If pointer to const then 'PC'.
109	*
110	* <li> Function typedefs start with 'FN'. If pointer to FN then 'PFN'.
111	*
112	* <li> All files are case sensitive.
113	*
114	* <li> Slashes are unix slashes ('/') runtime converts when necessary.
115	*
116	* <li> char strings are UTF-8.
117	*
118	* <li> Strings from any external source must be treated with utmost care as
119	* they do not have to be valid UTF-8. Only trust internal strings.
120	*
121	* <li> All functions return VBox status codes. There are three general
122	* exceptions from this:
123	*
124	* <ol>
125	* <li>Predicate functions. These are function which are boolean in
126	* nature and usage. They return bool. The function name will
127	* include 'Has', 'Is' or similar.
128	* <li>Functions which by nature cannot possibly fail.
129	* These return void.
130	* <li>"Get"-functions which return what they ask for.
131	* A get function becomes a "Query" function if there is any
132	* doubt about getting what is ask for.
133	* </ol>
134	*
135	* <li> VBox status codes have three subdivisions:
136	* <ol>
137	* <li> Errors, which are VERR_ prefixed and negative.
138	* <li> Warnings, which are VWRN_ prefixed and positive.
139	* <li> Informational, which are VINF_ prefixed and positive.
140	* </ol>
141	*
142	* <li> Platform/OS operation are generalized and put in the IPRT.
143	*
144	* <li> Other useful constructs are also put in the IPRT.
145	*
146	* <li> The code shall not cause compiler warnings. Check this on ALL
147	* the platforms.
148	*
149	* <li> The use of symbols leading with single or double underscores is
150	* forbidden as that intrudes on reserved compiler/system namespace. (3)
151	*
152	* <li> All files have file headers with $Id and a file tag which describes
153	* the file in a sentence or two.
154	* Note: Use the svn-ps.cmd/svn-ps.sh utility with the -a option to add
155	* new sources with keyword expansion and exporting correctly
156	* configured.
157	*
158	* <li> All public functions are fully documented in Doxygen style using the
159	* javadoc dialect (using the 'at' instead of the 'slash' as
160	* commandprefix.)
161	*
162	* <li> All structures in header files are described, including all their
163	* members. (Doxygen style, of course.)
164	*
165	* <li> All modules have a documentation '\@page' in the main source file
166	* which describes the intent and actual implementation.
167	*
168	* <li> Code which is doing things that are not immediately comprehensible
169	* shall include explanatory comments.
170	*
171	* <li> Documentation and comments are kept up to date.
172	*
173	* <li> Headers in /include/VBox shall not contain any slash-slash C++
174	* comments, only ANSI C comments!
175	*
176	* <li> Comments on \#else indicates what begins while the comment on a
177	* \#endif indicates what ended. Only add these when there are more than
178	* a few lines (6-10) of \#ifdef'ed code, otherwise they're just clutter.
179	*
180	* <li> \#ifdefs around a single function shall be tight, i.e. no empty
181	* lines between it and the function documentation and body.
182	*
183	* <li> \#ifdefs around more than one function shall be relaxed, i.e. leave at
184	* least one line before the first function's documentation comment and
185	* one line after the end of the last function.
186	*
187	* <li> No 'else' after if block ending with 'return', 'break', or 'continue'.
188	*
189	* <li> The term 'last' is inclusive, whereas the term 'end' is exclusive.
190	*
191	* <li> Go through all of this: https://www.slideshare.net/olvemaudal/deep-c/
192	*
193	* </ul>
194	*
195	* (1) It is common practice on Unix to have a single symbol namespace for an
196	* entire process. If one is careless symbols might be resolved in a
197	* different way that one expects, leading to weird problems.
198	*
199	* (2) This is common practice among most projects dealing with modules in
200	* shared libraries. The Windows / PE __declspect(import) and
201	* __declspect(export) constructs are the main reason for this.
202	* OTOH, we do perhaps have a bit too detailed graining of this in VMM...
203	*
204	* (3) There are guys out there grepping public sources for symbols leading with
205	* single and double underscores as well as gotos and other things
206	* considered bad practice. They'll post statistics on how bad our sources
207	* are on some mailing list, forum or similar.
208	*
209	*
210	* @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit
211	*
212	* Here are some amendments which address 64-bit vs. 32-bit portability issues.
213	*
214	* Some facts first:
215	*
216	* <ul>
217	*
218	* <li> On 64-bit Windows the type long remains 32-bit. On nearly all other
219	* 64-bit platforms long is 64-bit.
220	*
221	* <li> On all 64-bit platforms we care about, int is 32-bit, short is 16 bit
222	* and char is 8-bit.
223	* (I don't know about any platforms yet where this isn't true.)
224	*
225	* <li> size_t, ssize_t, uintptr_t, ptrdiff_t and similar are all 64-bit on
226	* 64-bit platforms. (These are 32-bit on 32-bit platforms.)
227	*
228	* <li> There is no inline assembly support in the 64-bit Microsoft compilers.
229	*
230	* </ul>
231	*
232	* Now for the guidelines:
233	*
234	* <ul>
235	*
236	* <li> Never, ever, use int, long, ULONG, LONG, DWORD or similar to cast a
237	* pointer to integer. Use uintptr_t or intptr_t. If you have to use
238	* NT/Windows types, there is the choice of ULONG_PTR and DWORD_PTR.
239	*
240	* <li> Avoid where ever possible the use of the types 'long' and 'unsigned
241	* long' as these differs in size between windows and the other hosts
242	* (see above).
243	*
244	* <li> RT_OS_WINDOWS is defined to indicate Windows. Do not use __WIN32__,
245	* __WIN64__ and __WIN__ because they are all deprecated and scheduled
246	* for removal (if not removed already). Do not use the compiler
247	* defined _WIN32, _WIN64, or similar either. The bitness can be
248	* determined by testing ARCH_BITS.
249	* Example:
250	* @code
251	* #ifdef RT_OS_WINDOWS
252	* // call win32/64 api.
253	* #endif
254	* #ifdef RT_OS_WINDOWS
255	* # if ARCH_BITS == 64
256	* // call win64 api.
257	* # else // ARCH_BITS == 32
258	* // call win32 api.
259	* # endif // ARCH_BITS == 32
260	* #else // !RT_OS_WINDOWS
261	* // call posix api
262	* #endif // !RT_OS_WINDOWS
263	* @endcode
264	*
265	* <li> There are RT_OS_xxx defines for each OS, just like RT_OS_WINDOWS
266	* mentioned above. Use these defines instead of any predefined
267	* compiler stuff or defines from system headers.
268	*
269	* <li> RT_ARCH_X86 is defined when compiling for the x86 the architecture.
270	* Do not use __x86__, __X86__, __[Ii]386__, __[Ii]586__, or similar
271	* for this purpose.
272	*
273	* <li> RT_ARCH_AMD64 is defined when compiling for the AMD64 architecture.
274	* Do not use __AMD64__, __amd64__ or __x64_86__.
275	*
276	* <li> Take care and use size_t when you have to, esp. when passing a pointer
277	* to a size_t as a parameter.
278	*
279	* <li> Be wary of type promotion to (signed) integer. For example the
280	* following will cause u8 to be promoted to int in the shift, and then
281	* sign extended in the assignment 64-bit:
282	* @code
283	* uint8_t u8 = 0xfe;
284	* uint64_t u64 = u8 << 24;
285	* // u64 == 0xfffffffffe000000
286	* @endcode
287	*
288	* </ul>
289	*
290	* @subsection sec_vbox_guideline_compulsory_cppmain C++ guidelines for Main
291	*
292	* Main is currently (2009) full of hard-to-maintain code that uses complicated
293	* templates. The new mid-term goal for Main is to have less custom templates
294	* instead of more for the following reasons:
295	*
296	* <ul>
297	*
298	* <li> Template code is harder to read and understand. Custom templates create
299	* territories which only the code writer understands.
300	*
301	* <li> Errors in using templates create terrible C++ compiler messages.
302	*
303	* <li> Template code is really hard to look at in a debugger.
304	*
305	* <li> Templates slow down the compiler a lot.
306	*
307	* </ul>
308	*
309	* In particular, the following bits should be considered deprecated and should
310	* NOT be used in new code:
311	*
312	* <ul>
313	*
314	* <li> everything in include/iprt/cpputils.h (auto_ref_ptr, exception_trap_base,
315	* char_auto_ptr and friends)
316	*
317	* </ul>
318	*
319	* Generally, in many cases, a simple class with a proper destructor can achieve
320	* the same effect as a 1,000-line template include file, and the code is
321	* much more accessible that way.
322	*
323	* Using standard STL templates like std::list, std::vector and std::map is OK.
324	* Exceptions are:
325	*
326	* <ul>
327	*
328	* <li> Guest Additions because we don't want to link against libstdc++ there.
329	*
330	* <li> std::string should not be used because we have iprt::MiniString and
331	* com::Utf8Str which can convert efficiently with COM's UTF-16 strings.
332	*
333	* <li> std::auto_ptr<> in general; that part of the C++ standard is just broken.
334	* Write a destructor that calls delete.
335	*
336	* </ul>
337	*
338	* @subsection sec_vbox_guideline_compulsory_cppqtgui C++ guidelines for the Qt GUI
339	*
340	* The Qt GUI is currently (2010) on its way to become more compatible to the
341	* rest of VirtualBox coding style wise. From now on, all the coding style
342	* rules described in this file are also mandatory for the Qt GUI. Additionally
343	* the following rules should be respected:
344	*
345	* <ul>
346	*
347	* <li> GUI classes which correspond to GUI tasks should be prefixed by UI (no VBox anymore)
348	*
349	* <li> Classes which extents some of the Qt classes should be prefix by QI
350	*
351	* <li> General task classes should be prefixed by C
352	*
353	* <li> Slots are prefixed by slt -> sltName
354	*
355	* <li> Signals are prefixed by sig -> sigName
356	*
357	* <li> Use Qt classes for lists, strings and so on, the use of STL classes should
358	* be avoided
359	*
360	* <li> All files like .cpp, .h, .ui, which belong together are located in the
361	* same directory and named the same
362	*
363	* </ul>
364	*
365	*
366	* @subsection sec_vbox_guideline_compulsory_xslt XSLT
367	*
368	* XSLT (eXtensible Stylesheet Language Transformations) is used quite a bit in
369	* the Main API area of VirtualBox to generate sources and bindings to that API.
370	* There are a couple of common pitfalls worth mentioning:
371	*
372	* <ul>
373	*
374	* <li> Never do repeated //interface[\@name=...] and //enum[\@name=...] lookups
375	* because they are expensive. Instead delcare xsl:key elements for these
376	* searches and do the lookup using the key() function. xsltproc uses
377	* (per current document) hash tables for each xsl:key, i.e. very fast.
378	*
379	* <li> When output type is 'text' make sure to call xsltprocNewlineOutputHack
380	* from typemap-shared.inc.xsl every few KB of output, or xsltproc will
381	* end up wasting all the time reallocating the output buffer.
382	*
383	* </ul>
384	*
385	*
386	* @subsection sec_vbox_guideline_compulsory_doxygen Doxygen Comments
387	*
388	* As mentioned above, we shall use doxygen/javadoc style commenting of public
389	* functions, typedefs, classes and such. It is mandatory to use this style
390	* everywhere!
391	*
392	* A couple of hints on how to best write doxygen comments:
393	*
394	* <ul>
395	*
396	* <li> A good class, method, function, structure or enum doxygen comment
397	* starts with a one line sentence giving a brief description of the
398	* item. Details comes in a new paragraph (after blank line).
399	*
400	* <li> Except for list generators like \@todo, \@cfgm, \@gcfgm and others,
401	* all doxygen comments are related to things in the code. So, for
402	* instance you DO NOT add a doxygen \@note comment in the middle of a
403	* because you've got something important to note, you add a normal
404	* comment like 'Note! blah, very importan blah!'
405	*
406	* <li> We do NOT use TODO/XXX/BUGBUG or similar markers in the code to flag
407	* things needing fixing later, we always use \@todo doxygen comments.
408	*
409	* <li> There is no colon after the \@todo. And it is ALWAYS in a doxygen
410	* comment.
411	*
412	* <li> The \@retval tag is used to explain status codes a method/function may
413	* returns. It is not used to describe output parameters, that is done
414	* using the \@param or \@param[out] tag.
415	*
416	* </ul>
417	*
418	* See https://www.stack.nl/~dimitri/doxygen/manual/index.html for the official
419	* doxygen documention.
420	*
421	*
422	*
423	* @subsection sec_vbox_guideline_compulsory_guest Handling of guest input
424	*
425	* First, guest input should ALWAYS be consider to be TOXIC and constructed with
426	* MALICIOUS intent! Max paranoia level!
427	*
428	* Second, when getting inputs from memory shared with the guest, be EXTREMELY
429	* careful to not re-read input from shared memory after validating it, because
430	* that will create TOCTOU problems. So, after reading input from shared memory
431	* always use the RT_UNTRUSTED_NONVOLATILE_COPY_FENCE() macro. For more details
432	* on TOCTOU: https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
433	*
434	* Thirdly, considering the recent speculation side channel issues, spectre v1
435	* in particular, we would like to be ready for future screwups. This means
436	* having input validation in a separate block of code that ends with one (or
437	* more) RT_UNTRUSTED_VALIDATED_FENCE().
438	*
439	* So the rules:
440	*
441	* <ul>
442	*
443	* <li> Mark all pointers to shared memory with RT_UNTRUSTED_VOLATILE_GUEST.
444	*
445	* <li> Copy volatile data into local variables or heap before validating
446	* them (see RT_COPY_VOLATILE() and RT_BCOPY_VOLATILE().
447	*
448	* <li> Place RT_UNTRUSTED_NONVOLATILE_COPY_FENCE() after a block copying
449	* volatile data.
450	*
451	* <li> Always validate untrusted inputs in a block ending with a
452	* RT_UNTRUSTED_VALIDATED_FENCE().
453	*
454	* <li> Use the ASSERT_GUEST_XXXX macros from VBox/AssertGuest.h to validate
455	* guest input. (Do NOT use iprt/assert.h macros.)
456	*
457	* <li> Validation of an input B may require using another input A to look up
458	* some data, in which case its necessary to insert an
459	* RT_UNTRUSTED_VALIDATED_FENCE() after validating A and before A is used
460	* for the lookup.
461	*
462	* For example A is a view identifier, idView, and B is an offset into
463	* the view's framebuffer area, offView. To validate offView (B) it is
464	* necessary to get the size of the views framebuffer region:
465	* @code
466	* uint32_t const idView = pReq->idView; // A
467	* uint32_t const offView = pReq->offView; // B
468	* RT_UNTRUSTED_NONVOLATILE_COPY_FENCE();
469	*
470	* ASSERT_GUEST_RETURN(idView < pThis->cView,
471	* VERR_INVALID_PARAMETER);
472	* RT_UNTRUSTED_VALIDATED_FENCE();
473	* const MYVIEW *pView = &pThis->aViews[idView];
474	* ASSERT_GUEST_RETURN(offView < pView->cbFramebufferArea,
475	* VERR_OUT_OF_RANGE);
476	* RT_UNTRUSTED_VALIDATED_FENCE();
477	* @endcode
478	*
479	* <li> Take care to make sure input check are not subject to integer overflow problems.
480	*
481	* For instance when validating an area, you must not just add cbDst + offDst
482	* and check against pThis->offEnd or something like that. Rather do:
483	* @code
484	* uint32_t const offDst = pReq->offDst;
485	* uint32_t const cbDst = pReq->cbDst;
486	* RT_UNTRUSTED_NONVOLATILE_COPY_FENCE();
487	*
488	* ASSERT_GUEST_RETURN( cbDst <= pThis->cbSrc
489	* && offDst < pThis->cbSrc - cbDst,
490	* VERR_OUT_OF_RANGE);
491	* RT_UNTRUSTED_VALIDATED_FENCE();
492	* @endcode
493	*
494	* <li> Input validation does not only apply to shared data cases, but also to
495	* I/O port and MMIO handlers.
496	*
497	* <li> Ditto for kernel drivers working with usermode inputs.
498	*
499	* </ul>
500	*
501	*
502	* Problem patterns:
503	* - https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
504	* - https://googleprojectzero.blogspot.de/2018/01/reading-privileged-memory-with-side.html
505	* (Variant 1 only).
506	* - https://en.wikipedia.org/wiki/Integer_overflow
507	*
508	*
509	*
510	* @section sec_vbox_guideline_optional Optional
511	*
512	* First part is the actual coding style and all the prefixes. The second part
513	* is a bunch of good advice.
514	*
515	*
516	* @subsection sec_vbox_guideline_optional_layout The code layout
517	*
518	* <ul>
519	*
520	* <li> Max line length is 130 chars. Exceptions are table-like
521	* code/initializers and Log*() statements (don't waste unnecessary
522	* vertical space on debug logging).
523	*
524	* <li> Comments should try stay within the usual 80 columns as these are
525	* denser and too long lines may be harder to read.
526	*
527	* <li> Curly brackets are not indented. Example:
528	* @code
529	* if (true)
530	* {
531	* Something1();
532	* Something2();
533	* }
534	* else
535	* {
536	* SomethingElse1().
537	* SomethingElse2().
538	* }
539	* @endcode
540	*
541	* <li> Space before the parentheses when it comes after a C keyword.
542	*
543	* <li> No space between argument and parentheses. Exception for complex
544	* expression. Example:
545	* @code
546	* if (PATMR3IsPatchGCAddr(pVM, GCPtr))
547	* @endcode
548	*
549	* <li> The else of an if is always the first statement on a line. (No curly
550	* stuff before it!)
551	*
552	* <li> else and if go on the same line if no { compound statement }
553	* follows the if. Example:
554	* @code
555	* if (fFlags & MYFLAGS_1)
556	* fFlags &= ~MYFLAGS_10;
557	* else if (fFlags & MYFLAGS_2)
558	* {
559	* fFlags &= ~MYFLAGS_MASK;
560	* fFlags \|= MYFLAGS_5;
561	* }
562	* else if (fFlags & MYFLAGS_3)
563	* @endcode
564	*
565	* <li> Slightly complex boolean expressions are split into multiple lines,
566	* putting the operators first on the line and indenting it all according
567	* to the nesting of the expression. The purpose is to make it as easy as
568	* possible to read. Example:
569	* @code
570	* if ( RT_SUCCESS(rc)
571	* \|\| (fFlags & SOME_FLAG))
572	* @endcode
573	*
574	* <li> When 'if' or 'while' statements gets long, the closing parentheses
575	* goes right below the opening parentheses. This may be applied to
576	* sub-expression. Example:
577	* @code
578	* if ( RT_SUCCESS(rc)
579	* \|\| ( fSomeStuff
580	* && fSomeOtherStuff
581	* && fEvenMoreStuff
582	* )
583	* \|\| SomePredicateFunction()
584	* )
585	* {
586	* ...
587	* }
588	* @endcode
589	*
590	* <li> The case is indented from the switch (to avoid having the braces for
591	* the 'case' at the same level as the 'switch' statement).
592	*
593	* <li> If a case needs curly brackets they contain the entire case, are not
594	* indented from the case, and the break or return is placed inside them.
595	* Example:
596	* @code
597	* switch (pCur->eType)
598	* {
599	* case PGMMAPPINGTYPE_PAGETABLES:
600	* {
601	* unsigned iPDE = pCur->GCPtr >> PGDIR_SHIFT;
602	* unsigned iPT = (pCur->GCPtrEnd - pCur->GCPtr) >> PGDIR_SHIFT;
603	* while (iPT-- > 0)
604	* if (pPD->a[iPDE + iPT].n.u1Present)
605	* return VERR_HYPERVISOR_CONFLICT;
606	* break;
607	* }
608	* }
609	* @endcode
610	*
611	* <li> In a do while construction, the while is on the same line as the
612	* closing "}" if any are used.
613	* Example:
614	* @code
615	* do
616	* {
617	* stuff;
618	* i--;
619	* } while (i > 0);
620	* @endcode
621	*
622	* <li> Comments are in C style. C++ style comments are used for temporary
623	* disabling a few lines of code.
624	*
625	* <li> No unnecessary parentheses in expressions (just don't over do this
626	* so that gcc / msc starts bitching). Find a correct C/C++ operator
627	* precedence table if needed.
628	*
629	* <li> 'for (;;)' is preferred over 'while (true)' and 'while (1)'.
630	*
631	* <li> Parameters are indented to the start parentheses when breaking up
632	* function calls, declarations or prototypes. (This is in line with
633	* how 'if', 'for' and 'while' statements are done as well.) Example:
634	* @code
635	* RTPROCESS hProcess;
636	* int rc = RTProcCreateEx(papszArgs[0],
637	* papszArgs,
638	* RTENV_DEFAULT,
639	* fFlags,
640	* NULL, // phStdIn
641	* NULL, // phStdOut
642	* NULL, // phStdErr
643	* NULL, // pszAsUser
644	* NULL, // pszPassword
645	* &hProcess);
646	* @endcode
647	*
648	* <li> That Dijkstra is dead is no excuse for using gotos.
649	*
650	* <li> Using do-while-false loops to avoid gotos is considered very bad form.
651	* They create hard to read code. They tend to be either too short (i.e.
652	* pointless) or way to long (split up the function already), making
653	* tracking the state is difficult and prone to bugs. Also, they cause
654	* the compiler to generate suboptimal code, because the break branches
655	* are by preferred over the main code flow (MSC has no branch hinting!).
656	* Instead, do make use the 130 columns (i.e. nested ifs) and split
657	* the code up into more functions!
658	*
659	* <li> Avoid code like
660	* @code
661	* int foo;
662	* int rc;
663	* ...
664	* rc = FooBar();
665	* if (RT_SUCCESS(rc))
666	* {
667	* foo = getFoo();
668	* ...
669	* pvBar = RTMemAlloc(sizeof(*pvBar));
670	* if (!pvBar)
671	* rc = VERR_NO_MEMORY;
672	* }
673	* if (RT_SUCCESS(rc))
674	* {
675	* buzz = foo;
676	* ...
677	* }
678	* @endcode
679	* The intention of such code is probably to save some horizontal space
680	* but unfortunately it's hard to read and the scope of certain varables
681	* (e.g. foo in this example) is not optimal. Better use the following
682	* style:
683	* @code
684	* int rc;
685	* ...
686	* rc = FooBar();
687	* if (RT_SUCCESS(rc))
688	* {
689	* int foo = getFoo();
690	* ...
691	* pvBar = RTMemAlloc(sizeof(*pvBar));
692	* if (pvBar)
693	* {
694	* buzz = foo;
695	* ...
696	* }
697	* else
698	* rc = VERR_NO_MEMORY;
699	* }
700	* @endcode
701	*
702	* </ul>
703	*
704	* @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes
705	*
706	* Prefixes are meant to provide extra context clues to a variable/member, we
707	* therefore avoid using prefixes that just indicating the type if a better
708	* choice is available.
709	*
710	*
711	* The prefixes:
712	*
713	* <ul>
714	*
715	* <li> The 'g_' (or 'g') prefix means a global variable, either on file or module level.
716	*
717	* <li> The 's_' (or 's') prefix means a static variable inside a function or
718	* class. This is not used for static variables on file level, use 'g_'
719	* for those (logical, right).
720	*
721	* <li> The 'm_' (or 'm') prefix means a class data member.
722	*
723	* In new code in Main, use "m_" (and common sense). As an exception,
724	* in Main, if a class encapsulates its member variables in an anonymous
725	* structure which is declared in the class, but defined only in the
726	* implementation (like this: 'class X { struct Data; Data *m; }'), then
727	* the pointer to that struct is called 'm' itself and its members then
728	* need no prefix, because the members are accessed with 'm->member'
729	* already which is clear enough.
730	*
731	* <li> The 'a_' prefix means a parameter (argument) variable. This is
732	* sometimes written 'a' in parts of the source code that does not use
733	* the array prefix.
734	*
735	* <li> The 'p' prefix means pointer. For instance 'pVM' is pointer to VM.
736	*
737	* <li> The 'r' prefix means that something is passed by reference.
738	*
739	* <li> The 'k' prefix means that something is a constant. For instance
740	* 'enum { kStuff };'. This is usually not used in combination with
741	* 'p', 'r' or any such thing, it's main main use is to make enums
742	* easily identifiable.
743	*
744	* <li> The 'a' prefix means array. For instance 'aPages' could be read as
745	* array of pages.
746	*
747	* <li> The 'c' prefix means count. For instance 'cbBlock' could be read,
748	* count of bytes in block. (1)
749	*
750	* <li> The 'cx' prefix means width (count of 'x' units).
751	*
752	* <li> The 'cy' prefix means height (count of 'y' units).
753	*
754	* <li> The 'x', 'y' and 'z' prefix refers to the x-, y- , and z-axis
755	* respectively.
756	*
757	* <li> The 'off' prefix means offset.
758	*
759	* <li> The 'i' or 'idx' prefixes usually means index. Although the 'i' one
760	* can sometimes just mean signed integer.
761	*
762	* <li> The 'i[1-9]+' prefix means a fixed bit size variable. Frequently
763	* used with the int[1-9]+_t types where the width is really important.
764	* In most cases 'i' is more appropriate. [type]
765	*
766	* <li> The 'e' (or 'enm') prefix means enum.
767	*
768	* <li> The 'u' prefix usually means unsigned integer. Exceptions follows.
769	*
770	* <li> The 'u[1-9]+' prefix means a fixed bit size variable. Frequently
771	* used with the uint[1-9]+_t types and with bitfields where the width is
772	* really important. In most cases 'u' or 'b' (byte) would be more
773	* appropriate. [type]
774	*
775	* <li> The 'b' prefix means byte or bytes. [type]
776	*
777	* <li> The 'f' prefix means flags. Flags are unsigned integers of some kind
778	* or booleans.
779	*
780	* <li> TODO: need prefix for real float. [type]
781	*
782	* <li> The 'rd' prefix means real double and is used for 'double' variables.
783	* [type]
784	*
785	* <li> The 'lrd' prefix means long real double and is used for 'long double'
786	* variables. [type]
787	*
788	* <li> The 'ch' prefix means a char, the (signed) char type. [type]
789	*
790	* <li> The 'wc' prefix means a wide/windows char, the RTUTF16 type. [type]
791	*
792	* <li> The 'uc' prefix means a Unicode Code point, the RTUNICP type. [type]
793	*
794	* <li> The 'uch' prefix means unsigned char. It's rarely used. [type]
795	*
796	* <li> The 'sz' prefix means zero terminated character string (array of
797	* chars). (UTF-8)
798	*
799	* <li> The 'wsz' prefix means zero terminated wide/windows character string
800	* (array of RTUTF16).
801	*
802	* <li> The 'usz' prefix means zero terminated Unicode string (array of
803	* RTUNICP).
804	*
805	* <li> The 'str' prefix means C++ string; either a std::string or, in Main,
806	* a Utf8Str or, in Qt, a QString. When used with 'p', 'r', 'a' or 'c'
807	* the first letter should be capitalized.
808	*
809	* <li> The 'bstr' prefix, in Main, means a UTF-16 Bstr. When used with 'p',
810	* 'r', 'a' or 'c' the first letter should be capitalized.
811	*
812	* <li> The 'pfn' prefix means pointer to function. Common usage is 'pfnCallback'
813	* and such like.
814	*
815	* <li> The 'psz' prefix is a combination of 'p' and 'sz' and thus means
816	* pointer to a zero terminated character string. (UTF-8)
817	*
818	* <li> The 'pcsz' prefix is used to indicate constant string pointers in
819	* parts of the code. Most code uses 'psz' for const and non-const
820	* string pointers, so please ignore this one.
821	*
822	* <li> The 'l' prefix means (signed) long. We try avoid using this,
823	* expecially with the 'LONG' types in Main as these are not 'long' on
824	* 64-bit non-Windows platforms and can cause confusion. Alternatives:
825	* 'i' or 'i32'. [type]
826	*
827	* <li> The 'ul' prefix means unsigned long. We try avoid using this,
828	* expecially with the 'ULONG' types in Main as these are not 'unsigned
829	* long' on 64-bit non-Windows platforms and can cause confusion.
830	* Alternatives: 'u' or 'u32'. [type]
831	*
832	* </ul>
833	*
834	* (1) Except in the occasional 'pcsz' prefix, the 'c' prefix is never ever
835	* used in the meaning 'const'.
836	*
837	*
838	* @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff
839	*
840	* <ul>
841	*
842	* <li> When writing code think as the reader.
843	*
844	* <li> When writing code think as the compiler. (2)
845	*
846	* <li> When reading code think as if it's full of bugs - find them and fix them.
847	*
848	* <li> Pointer within range tests like:
849	* @code
850	* if ((uintptr_t)pv >= (uintptr_t)pvBase && (uintptr_t)pv < (uintptr_t)pvBase + cbRange)
851	* @endcode
852	* Can also be written as (assuming cbRange unsigned):
853	* @code
854	* if ((uintptr_t)pv - (uintptr_t)pvBase < cbRange)
855	* @endcode
856	* Which is shorter and potentially faster. (1)
857	*
858	* <li> Avoid unnecessary casting. All pointers automatically cast down to
859	* void *, at least for non class instance pointers.
860	*
861	* <li> It's very very bad practise to write a function larger than a
862	* screen full (1024x768) without any comprehensibility and explaining
863	* comments.
864	*
865	* <li> More to come....
866	*
867	* </ul>
868	*
869	* (1) Important, be very careful with the casting. In particular, note that
870	* a compiler might treat pointers as signed (IIRC).
871	*
872	* (2) "A really advanced hacker comes to understand the true inner workings of
873	* the machine - he sees through the language he's working in and glimpses
874	* the secret functioning of the binary code - becomes a Ba'al Shem of
875	* sorts." (Neal Stephenson "Snow Crash")
876	*
877	*
878	*
879	* @section sec_vbox_guideline_warnings Compiler Warnings
880	*
881	* The code should when possible compile on all platforms and compilers without any
882	* warnings. That's a nice idea, however, if it means making the code harder to read,
883	* less portable, unreliable or similar, the warning should not be fixed.
884	*
885	* Some of the warnings can seem kind of innocent at first glance. So, let's take the
886	* most common ones and explain them.
887	*
888	*
889	* @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare
890	*
891	* GCC says: "warning: comparison between signed and unsigned integer expressions"
892	* MSC says: "warning C4018: '<\|<=\|==\|>=\|>' : signed/unsigned mismatch"
893	*
894	* The following example will not output what you expect:
895	@code
896	#include <stdio.h>
897	int main()
898	{
899	signed long a = -1;
900	unsigned long b = 2294967295;
901	if (a < b)
902	printf("%ld < %lu: true\n", a, b);
903	else
904	printf("%ld < %lu: false\n", a, b);
905	return 0;
906	}
907	@endcode
908	* If I understood it correctly, the compiler will convert a to an
909	* unsigned long before doing the compare.
910	*
911	*
912	*
913	* @section sec_vbox_guideline_svn Subversion Commit Rules
914	*
915	*
916	* Before checking in:
917	*
918	* <ul>
919	*
920	* <li> Check Tinderbox and make sure the tree is green across all platforms. If it's
921	* red on a platform, don't check in. If you want, warn in the \#vbox channel and
922	* help make the responsible person fix it.
923	* NEVER CHECK IN TO A BROKEN BUILD.
924	*
925	* <li> When checking in keep in mind that a commit is atomic and that the Tinderbox and
926	* developers are constantly checking out the tree. Therefore do not split up the
927	* commit unless it's into 100% independent parts. If you need to split it up in order
928	* to have sensible commit comments, make the sub-commits as rapid as possible.
929	*
930	* <li> If you make a user visible change, such as fixing a reported bug,
931	* make sure you add an entry to doc/manual/user_ChangeLogImpl.xml.
932	*
933	* <li> If you are adding files make sure set the right attributes.
934	* svn-ps.sh/cmd was created for this purpose, please make use of it.
935	*
936	* </ul>
937	*
938	* After checking in:
939	*
940	* <ul>
941	*
942	* <li> After checking-in, you watch Tinderbox until your check-ins clear. You do not
943	* go home. You do not sleep. You do not log out or experiment with drugs. You do
944	* not become unavailable. If you break the tree, add a comment saying that you're
945	* fixing it. If you can't fix it and need help, ask in the \#innotek channel or back
946	* out the change.
947	*
948	* </ul>
949	*
950	* (Inspired by mozilla tree rules.)
951	*
952	*
953	*/
954

Note: See TracBrowser for help on using the repository browser.

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

Download in other formats: