/* $Id: VBox-CodingGuidelines.cpp 80569 2019-09-03 14:34:21Z vboxsync $ */ /** @file * VBox - Coding Guidelines. */ /* * Copyright (C) 2006-2019 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as * available from http://www.virtualbox.org. This file is free software; * you can redistribute it and/or modify it under the terms of the GNU * General Public License (GPL) as published by the Free Software * Foundation, in version 2 as it comes in the "COPYING" file of the * VirtualBox OSE distribution. VirtualBox OSE is distributed in the * hope that it will be useful, but WITHOUT ANY WARRANTY of any kind. */ /** @page pg_vbox_guideline VBox Coding Guidelines * * The compulsory sections of these guidelines are to be followed in all of the * VBox sources. Please note that local guidelines in parts of the VBox source * tree may promote the optional ones to compulsory status. The VBox tree also * contains some 3rd party sources where it is good to follow the local coding * style while keeping these guidelines in mind. * * Contents: * - @ref sec_vbox_guideline_compulsory * - @ref sec_vbox_guideline_compulsory_sub64 * - @ref sec_vbox_guideline_compulsory_cppmain * - @ref sec_vbox_guideline_compulsory_cppqtgui * - @ref sec_vbox_guideline_compulsory_xslt * - @ref sec_vbox_guideline_compulsory_doxygen * - @ref sec_vbox_guideline_compulsory_guest * - @ref sec_vbox_guideline_optional * - @ref sec_vbox_guideline_optional_layout * - @ref sec_vbox_guideline_optional_prefix * - @ref sec_vbox_guideline_optional_misc * - @ref sec_vbox_guideline_warnings * - @ref sec_vbox_guideline_warnings_signed_unsigned_compare * - @ref sec_vbox_guideline_svn * * Local guidelines overrides: * - src/VBox/VMM/: @ref pg_vmm_guideline (src/VBox/VMM/Docs-CodingGuidelines.cpp) * - src/VBox/ValidationKit/: @ref pg_validationkit_guideline (src/VBox/ValidationKit/ValidationKitCodingGuidelines.cpp) * - src/VBox/Runtime/: All of @ref sec_vbox_guideline_optional is mandatory. * - src/VBox/Main/: @ref sec_vbox_guideline_compulsory_cppmain * - src/VBox/Frontends/VirtualBox/: @ref sec_vbox_guideline_compulsory_cppqtgui * * * @section sec_vbox_guideline_compulsory Compulsory * * * * (1) It is common practice on Unix to have a single symbol namespace for an * entire process. If one is careless symbols might be resolved in a * different way that one expects, leading to weird problems. * * (2) This is common practice among most projects dealing with modules in * shared libraries. The Windows / PE __declspect(import) and * __declspect(export) constructs are the main reason for this. * OTOH, we do perhaps have a bit too detailed graining of this in VMM... * * (3) There are guys out there grepping public sources for symbols leading with * single and double underscores as well as gotos and other things * considered bad practice. They'll post statistics on how bad our sources * are on some mailing list, forum or similar. * * * @subsection sec_vbox_guideline_compulsory_sub64 64-bit and 32-bit * * Here are some amendments which address 64-bit vs. 32-bit portability issues. * * Some facts first: * * * * Now for the guidelines: * * * * @subsection sec_vbox_guideline_compulsory_cppmain C++ guidelines for Main * * Since the Main API code is a large amount of C++ code, it is allowed but * not required to use C++ style comments (as permanent comments, beyond the * temporary use allowed by the general coding guideline). This is a weak * preference, i.e. large scale comment style changes are not encouraged. * * Main is currently (2009) full of hard-to-maintain code that uses complicated * templates. The new mid-term goal for Main is to have less custom templates * instead of more for the following reasons: * * * * In particular, the following bits should be considered deprecated and should * NOT be used in new code: * * * * Generally, in many cases, a simple class with a proper destructor can achieve * the same effect as a 1,000-line template include file, and the code is * much more accessible that way. * * Using standard STL templates like std::list, std::vector and std::map is OK. * Exceptions are: * * * * @subsection sec_vbox_guideline_compulsory_cppqtgui C++ guidelines for the Qt GUI * * The Qt GUI is currently (2010) on its way to become more compatible to the * rest of VirtualBox coding style wise. From now on, all the coding style * rules described in this file are also mandatory for the Qt GUI. Additionally * the following rules should be respected: * * * * * @subsection sec_vbox_guideline_compulsory_xslt XSLT * * XSLT (eXtensible Stylesheet Language Transformations) is used quite a bit in * the Main API area of VirtualBox to generate sources and bindings to that API. * There are a couple of common pitfalls worth mentioning: * * * * * @subsection sec_vbox_guideline_compulsory_doxygen Doxygen Comments * * As mentioned above, we shall use doxygen/javadoc style commenting of public * functions, typedefs, classes and such. It is mandatory to use this style * everywhere! * * A couple of hints on how to best write doxygen comments: * * * * See https://www.stack.nl/~dimitri/doxygen/manual/index.html for the official * doxygen documention. * * * * @subsection sec_vbox_guideline_compulsory_guest Handling of guest input * * First, guest input should ALWAYS be consider to be TOXIC and constructed with * MALICIOUS intent! Max paranoia level! * * Second, when getting inputs from memory shared with the guest, be EXTREMELY * careful to not re-read input from shared memory after validating it, because * that will create TOCTOU problems. So, after reading input from shared memory * always use the RT_UNTRUSTED_NONVOLATILE_COPY_FENCE() macro. For more details * on TOCTOU: https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use * * Thirdly, considering the recent speculation side channel issues, spectre v1 * in particular, we would like to be ready for future screwups. This means * having input validation in a separate block of code that ends with one (or * more) RT_UNTRUSTED_VALIDATED_FENCE(). * * So the rules: * * * * * Problem patterns: * - https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use * - https://googleprojectzero.blogspot.de/2018/01/reading-privileged-memory-with-side.html * (Variant 1 only). * - https://en.wikipedia.org/wiki/Integer_overflow * * * * @section sec_vbox_guideline_optional Optional * * First part is the actual coding style and all the prefixes. The second part * is a bunch of good advice. * * * @subsection sec_vbox_guideline_optional_layout The code layout * * * * @subsection sec_vbox_guideline_optional_prefix Variable / Member Prefixes * * Prefixes are meant to provide extra context clues to a variable/member, we * therefore avoid using prefixes that just indicating the type if a better * choice is available. * * * The prefixes: * * * * (1) Except in the occasional 'pcsz' prefix, the 'c' prefix is never ever * used in the meaning 'const'. * * * @subsection sec_vbox_guideline_optional_misc Misc / Advice / Stuff * * * * (1) Important, be very careful with the casting. In particular, note that * a compiler might treat pointers as signed (IIRC). * * (2) "A really advanced hacker comes to understand the true inner workings of * the machine - he sees through the language he's working in and glimpses * the secret functioning of the binary code - becomes a Ba'al Shem of * sorts." (Neal Stephenson "Snow Crash") * * * * @section sec_vbox_guideline_warnings Compiler Warnings * * The code should when possible compile on all platforms and compilers without any * warnings. That's a nice idea, however, if it means making the code harder to read, * less portable, unreliable or similar, the warning should not be fixed. * * Some of the warnings can seem kind of innocent at first glance. So, let's take the * most common ones and explain them. * * * @subsection sec_vbox_guideline_warnings_signed_unsigned_compare Signed / Unsigned Compare * * GCC says: "warning: comparison between signed and unsigned integer expressions" * MSC says: "warning C4018: '<|<=|==|>=|>' : signed/unsigned mismatch" * * The following example will not output what you expect: @code #include int main() { signed long a = -1; unsigned long b = 2294967295; if (a < b) printf("%ld < %lu: true\n", a, b); else printf("%ld < %lu: false\n", a, b); return 0; } @endcode * If I understood it correctly, the compiler will convert a to an * unsigned long before doing the compare. * * * * @section sec_vbox_guideline_svn Subversion Commit Rules * * * Before checking in: * * * * After checking in: * * * * (Inspired by mozilla tree rules.) * * */