VBox-CodingGuidelines.cpp@ 103416

Last change on this file since 103416 was 103312, checked in by vboxsync, 9 months ago
docs/VBox-CodingGuidelines: Added warning about type promotion issues when using bitfield values in calculations.
Property svn:eol-style set to `native` Property svn:keywords set to `Author Date Id Revision`
File size: 40.4 KB

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

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

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

Download in other formats: