VBox-CodingGuidelines.cpp@ 98785

Last change on this file since 98785 was 98103, checked in by vboxsync, 22 months 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.9 KB

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

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

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

Download in other formats: