VirtualBox

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

Last change on this file since 78746 was 78746, checked in by vboxsync, 5 years ago

VBox-CodingGuidlines.cpp: Added a general guideline reading exceessive and bad exception handling.

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

© 2023 Oracle
ContactPrivacy policyTerms of Use