VBox-CodingGuidelines.cpp@ 87835

Last change on this file since 87835 was 82968, checked in by vboxsync, 5 years ago
Copyright year updates by scm.
Property svn:eol-style set to `native` Property svn:keywords set to `Author Date Id Revision`
File size: 39.6 KB

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

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

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

Download in other formats: