| <!-- HTML header for doxygen 1.8.15--> |
| <!-- Remember to use version doxygen 1.8.15 +--> |
| <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> |
| <html xmlns="http://www.w3.org/1999/xhtml"> |
| <head> |
| <meta http-equiv="Content-Type" content="text/xhtml;charset=UTF-8"/> |
| <meta http-equiv="X-UA-Compatible" content="IE=9"/> |
| <meta name="generator" content="Doxygen 1.8.15"/> |
| <meta name="robots" content="NOINDEX, NOFOLLOW" /> <!-- Prevent indexing by search engines --> |
| <title>Compute Library: Contribution guidelines</title> |
| <link href="tabs.css" rel="stylesheet" type="text/css"/> |
| <script type="text/javascript" src="jquery.js"></script> |
| <script type="text/javascript" src="dynsections.js"></script> |
| <link href="navtree.css" rel="stylesheet" type="text/css"/> |
| <script type="text/javascript" src="resize.js"></script> |
| <script type="text/javascript" src="navtreedata.js"></script> |
| <script type="text/javascript" src="navtree.js"></script> |
| <script type="text/javascript"> |
| /* @license magnet:?xt=urn:btih:cf05388f2679ee054f2beb29a391d25f4e673ac3&dn=gpl-2.0.txt GPL-v2 */ |
| $(document).ready(initResizable); |
| /* @license-end */</script> |
| <link href="search/search.css" rel="stylesheet" type="text/css"/> |
| <script type="text/javascript" src="search/searchdata.js"></script> |
| <script type="text/javascript" src="search/search.js"></script> |
| <script type="text/x-mathjax-config"> |
| MathJax.Hub.Config({ |
| extensions: ["tex2jax.js"], |
| jax: ["input/TeX","output/HTML-CSS"], |
| }); |
| </script><script type="text/javascript" async="async" src="http://cdn.mathjax.org/mathjax/latest/MathJax.js"></script> |
| <link href="doxygen.css" rel="stylesheet" type="text/css" /> |
| <link href="stylesheet.css" rel="stylesheet" type="text/css"/> |
| </head> |
| <body> |
| <div id="top"><!-- do not remove this div, it is closed by doxygen! --> |
| <div id="titlearea"> |
| <table cellspacing="0" cellpadding="0"> |
| <tbody> |
| <tr style="height: 56px;"> |
| <img alt="Compute Library" src="https://raw.githubusercontent.com/ARM-software/ComputeLibrary/gh-pages/ACL_logo.png" style="max-width: 100%;margin-top: 15px;margin-left: 10px"/> |
| <td style="padding-left: 0.5em;"> |
| <div id="projectname"> |
|  <span id="projectnumber">20.02.1</span> |
| </div> |
| </td> |
| </tr> |
| </tbody> |
| </table> |
| </div> |
| <!-- end header part --> |
| <!-- Generated by Doxygen 1.8.15 --> |
| <script type="text/javascript"> |
| /* @license magnet:?xt=urn:btih:cf05388f2679ee054f2beb29a391d25f4e673ac3&dn=gpl-2.0.txt GPL-v2 */ |
| var searchBox = new SearchBox("searchBox", "search",false,'Search'); |
| /* @license-end */ |
| </script> |
| <script type="text/javascript" src="menudata.js"></script> |
| <script type="text/javascript" src="menu.js"></script> |
| <script type="text/javascript"> |
| /* @license magnet:?xt=urn:btih:cf05388f2679ee054f2beb29a391d25f4e673ac3&dn=gpl-2.0.txt GPL-v2 */ |
| $(function() { |
| initMenu('',true,false,'search.php','Search'); |
| $(document).ready(function() { init_search(); }); |
| }); |
| /* @license-end */</script> |
| <div id="main-nav"></div> |
| </div><!-- top --> |
| <div id="side-nav" class="ui-resizable side-nav-resizable"> |
| <div id="nav-tree"> |
| <div id="nav-tree-contents"> |
| <div id="nav-sync" class="sync"></div> |
| </div> |
| </div> |
| <div id="splitbar" style="-moz-user-select:none;" |
| class="ui-resizable-handle"> |
| </div> |
| </div> |
| <script type="text/javascript"> |
| /* @license magnet:?xt=urn:btih:cf05388f2679ee054f2beb29a391d25f4e673ac3&dn=gpl-2.0.txt GPL-v2 */ |
| $(document).ready(function(){initNavTree('contribution_guidelines.xhtml','');}); |
| /* @license-end */ |
| </script> |
| <div id="doc-content"> |
| <!-- window showing the filter options --> |
| <div id="MSearchSelectWindow" |
| onmouseover="return searchBox.OnSearchSelectShow()" |
| onmouseout="return searchBox.OnSearchSelectHide()" |
| onkeydown="return searchBox.OnSearchSelectKey(event)"> |
| </div> |
| |
| <!-- iframe showing the search results (closed by default) --> |
| <div id="MSearchResultsWindow"> |
| <iframe src="javascript:void(0)" frameborder="0" |
| name="MSearchResults" id="MSearchResults"> |
| </iframe> |
| </div> |
| |
| <div class="PageDoc"><div class="header"> |
| <div class="headertitle"> |
| <div class="title">Contribution guidelines </div> </div> |
| </div><!--header--> |
| <div class="contents"> |
| <div class="toc"><h3>Table of Contents</h3> |
| <ul><li class="level1"><a href="#S5_1_coding_standards">Coding standards and guidelines</a><ul><li class="level2"><a href="#S5_1_1_rules">Rules</a></li> |
| <li class="level2"><a href="#S5_1_2_how_to_check_the_rules">How to check the rules</a></li> |
| <li class="level2"><a href="#S5_1_3_library_size_guidelines">Library size: best practices and guidelines</a><ul><li class="level3"><a href="#S5_1_3_1_template_suggestions">Template suggestions</a></li> |
| </ul> |
| </li> |
| <li class="level2"><a href="#S5_1_4_secure_coding_practices">Secure coding practices</a><ul><li class="level3"><a href="#S5_1_4_1_general_coding_practices">General Coding Practices</a></li> |
| <li class="level3"><a href="#S5_1_4_2_secure_coding_best_practices">Secure Coding Best Practices</a></li> |
| </ul> |
| </li> |
| </ul> |
| </li> |
| <li class="level1"><a href="#S5_2_how_to_submit_a_patch">How to submit a patch</a></li> |
| <li class="level1"><a href="#S5_3_code_review">Patch acceptance and code review</a></li> |
| </ul> |
| </div> |
| <div class="textblock"><p>If you want to contribute to Arm Compute Library, be sure to review the following guidelines.</p> |
| <p>The development is structured in the following way:</p><ul> |
| <li>Release repository: <a href="https://github.com/arm-software/ComputeLibrary">https://github.com/arm-software/ComputeLibrary</a></li> |
| <li>Development repository: <a href="https://review.mlplatform.org/#/admin/projects/ml/ComputeLibrary">https://review.mlplatform.org/#/admin/projects/ml/ComputeLibrary</a></li> |
| <li>Please report issues here: <a href="https://github.com/ARM-software/ComputeLibrary/issues">https://github.com/ARM-software/ComputeLibrary/issues</a></li> |
| </ul> |
| <h1><a class="anchor" id="S5_1_coding_standards"></a> |
| Coding standards and guidelines</h1> |
| <p>Best practices (as suggested by clang-tidy):</p> |
| <ul> |
| <li>No uninitialised values</li> |
| </ul> |
| <p>Helps to prevent undefined behaviour and allows to declare variables const if they are not changed after initialisation. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.html">http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.html</a></p> |
| <div class="fragment"><div class="line"><span class="keyword">const</span> float32x4_t foo = vdupq_n_f32(0.f);</div><div class="line"><span class="keyword">const</span> float32x4_t bar = foo;</div><div class="line"></div><div class="line"><span class="keyword">const</span> int32x4x2_t i_foo = {{</div><div class="line"> vconvq_s32_f32(foo),</div><div class="line"> vconvq_s32_f32(foo)</div><div class="line">}};</div><div class="line"><span class="keyword">const</span> int32x4x2_t i_bar = i_foo;</div></div><!-- fragment --><ul> |
| <li>No C-style casts (in C++ source code)</li> |
| </ul> |
| <p>Only use static_cast, dynamic_cast, and (if required) reinterpret_cast and const_cast. See <a href="http://en.cppreference.com/w/cpp/language/explicit_cast">http://en.cppreference.com/w/cpp/language/explicit_cast</a> for more information when to use which type of cast. C-style casts do not differentiate between the different cast types and thus make it easy to violate type safety. Also, due to the prefix notation it is less clear which part of an expression is going to be casted. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.html">http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.html</a></p> |
| <ul> |
| <li>No implicit casts to bool</li> |
| </ul> |
| <p>Helps to increase readability and might help to catch bugs during refactoring. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-cast.html">http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-cast.html</a></p> |
| <div class="fragment"><div class="line"><span class="keyword">extern</span> <span class="keywordtype">int</span> *ptr;</div><div class="line"><span class="keywordflow">if</span>(ptr){} <span class="comment">// Bad</span></div><div class="line"><span class="keywordflow">if</span>(ptr != <span class="keyword">nullptr</span>) {} <span class="comment">// Good</span></div><div class="line"></div><div class="line"><span class="keyword">extern</span> <span class="keywordtype">int</span> foo;</div><div class="line"><span class="keywordflow">if</span>(foo) {} <span class="comment">// Bad</span></div><div class="line"><span class="keywordflow">if</span>(foo != 0) {} <span class="comment">// Good</span></div></div><!-- fragment --><ul> |
| <li>Use nullptr instead of NULL or 0</li> |
| </ul> |
| <p>The nullptr literal is type-checked and is therefore safer to use. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html">http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html</a></p> |
| <ul> |
| <li>No need to explicitly initialise std::string with an empty string</li> |
| </ul> |
| <p>The default constructor of std::string creates an empty string. In general it is therefore not necessary to specify it explicitly. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-init.html">http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-init.html</a></p> |
| <div class="fragment"><div class="line"><span class="comment">// Instead of</span></div><div class="line">std::string foo(<span class="stringliteral">""</span>);</div><div class="line">std::string bar = <span class="stringliteral">""</span>;</div><div class="line"></div><div class="line"><span class="comment">// The following has the same effect</span></div><div class="line">std::string foo;</div><div class="line">std::string bar;</div></div><!-- fragment --><ul> |
| <li>Braces for all control blocks and loops (which have a body)</li> |
| </ul> |
| <p>To increase readability and protect against refactoring errors the body of control block and loops must be wrapped in braces. See <a href="http://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html">http://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html</a></p> |
| <p>For now loops for which the body is empty do not have to add empty braces. This exception might be revoked in the future. Anyway, situations in which this exception applies should be rare.</p> |
| <div class="fragment"><div class="line">Iterator it;</div><div class="line"><span class="keywordflow">while</span>(it.next()); <span class="comment">// No need for braces here</span></div><div class="line"></div><div class="line"><span class="comment">// Make more use of it</span></div></div><!-- fragment --><ul> |
| <li>Only one declaration per line</li> |
| </ul> |
| <p>Increase readability and thus prevent errors.</p> |
| <div class="fragment"><div class="line"><span class="keywordtype">int</span> a, <a class="code" href="namespacearm__compute_1_1test_1_1validation.xhtml#aa76b4a6e74940dabc5b7fc6b2dab3545">b</a>; <span class="comment">// BAD</span></div><div class="line"><span class="keywordtype">int</span> c, *d; <span class="comment">// EVEN WORSE</span></div><div class="line"></div><div class="line"><span class="keywordtype">int</span> e = 0; <span class="comment">// GOOD</span></div><div class="line"><span class="keywordtype">int</span> *p = <span class="keyword">nullptr</span>; <span class="comment">// GOOD</span></div></div><!-- fragment --><ul> |
| <li>Pass primitive types (and those that are cheap to copy or move) by value</li> |
| </ul> |
| <p>For primitive types it is more efficient to pass them by value instead of by const reference because:</p> |
| <ul> |
| <li>the data type might be smaller than the "reference type"</li> |
| <li>pass by value avoids aliasing and thus allows for better optimisations</li> |
| <li>pass by value is likely to avoid one level of indirection (references are often implemented as auto dereferenced pointers)</li> |
| </ul> |
| <p>This advice also applies to non-primitive types that have cheap copy or move operations and the function needs a local copy of the argument anyway.</p> |
| <p>More information:</p> |
| <ul> |
| <li><a href="http://stackoverflow.com/a/14013189">http://stackoverflow.com/a/14013189</a></li> |
| <li><a href="http://stackoverflow.com/a/270435">http://stackoverflow.com/a/270435</a></li> |
| <li><a href="http://web.archive.org/web/20140113221447/http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/">http://web.archive.org/web/20140113221447/http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/</a></li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keywordtype">void</span> foo(<span class="keywordtype">int</span> i, <span class="keywordtype">long</span> l, float32x4_t f); <span class="comment">// Pass-by-value for builtin types</span></div><div class="line"><span class="keywordtype">void</span> bar(<span class="keyword">const</span> float32x4x4_t &f); <span class="comment">// As this is a struct pass-by-const-reference is probably better</span></div><div class="line"><span class="keywordtype">void</span> foobar(<span class="keyword">const</span> MyLargeCustomTypeClass &m); <span class="comment">// Definitely better as const-reference except if a copy has to be made anyway.</span></div></div><!-- fragment --><ul> |
| <li>Don't use unions</li> |
| </ul> |
| <p>Unions cannot be used to convert values between different types because (in C++) it is undefined behaviour to read from a member other than the last one that has been assigned to. This limits the use of unions to a few corner cases and therefor the general advice is not to use unions. See <a href="http://releases.llvm.org/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-union-access.html">http://releases.llvm.org/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-union-access.html</a></p> |
| <ul> |
| <li>Use pre-increment/pre-decrement whenever possible</li> |
| </ul> |
| <p>In contrast to the pre-incerement the post-increment has to make a copy of the incremented object. This might not be a problem for primitive types like int but for class like objects that overload the operators, like iterators, it can have a huge impact on the performance. See <a href="http://stackoverflow.com/a/9205011">http://stackoverflow.com/a/9205011</a></p> |
| <p>To be consistent across the different cases the general advice is to use the pre-increment operator unless post-increment is explicitly required. The same rules apply for the decrement operator.</p> |
| <div class="fragment"><div class="line"><span class="keywordflow">for</span>(<span class="keywordtype">size_t</span> i = 0; i < 9; i++); <span class="comment">// BAD</span></div><div class="line"><span class="keywordflow">for</span>(<span class="keywordtype">size_t</span> i = 0; i < 9; ++i); <span class="comment">// GOOD</span></div></div><!-- fragment --><ul> |
| <li>Don't use uint in C/C++</li> |
| </ul> |
| <p>The C and C++ standards don't define a uint type. Though some compilers seem to support it by default it would require to include the header sys/types.h. Instead we use the slightly more verbose unsigned int type.</p> |
| <ul> |
| <li>Don't use unsigned int in function's signature</li> |
| </ul> |
| <p>Unsigned integers are good for representing bitfields and modular arithmetic. The fact that unsigned arithmetic doesn't model the behavior of a simple integer, but is instead defined by the standard to model modular arithmetic (wrapping around on overflow/underflow), means that a significant class of bugs cannot be diagnosed by the compiler. Mixing signedness of integer types is responsible for an equally large class of problems.</p> |
| <ul> |
| <li>No "Yoda-style" comparisons</li> |
| </ul> |
| <p>As compilers are now able to warn about accidental assignments if it is likely that the intention has been to compare values it is no longer required to place literals on the left-hand side of the comparison operator. Sticking to the natural order increases the readability and thus prevents logical errors (which cannot be spotted by the compiler). In the rare case that the desired result is to assign a value and check it the expression has to be surrounded by parentheses.</p> |
| <div class="fragment"><div class="line"><span class="keywordflow">if</span>(<span class="keyword">nullptr</span> == ptr || <span class="keyword">false</span> == cond) <span class="comment">// BAD</span></div><div class="line">{</div><div class="line"> <span class="comment">//...</span></div><div class="line">}</div><div class="line"></div><div class="line"><span class="keywordflow">if</span>(ptr == <span class="keyword">nullptr</span> || cond == <span class="keyword">false</span>) <span class="comment">// GOOD</span></div><div class="line">{</div><div class="line"> <span class="comment">//...</span></div><div class="line">}</div><div class="line"></div><div class="line"><span class="keywordflow">if</span>(ptr = <span class="keyword">nullptr</span> || cond = <span class="keyword">false</span>) <span class="comment">// Most likely a mistake. Will cause a compiler warning</span></div><div class="line">{</div><div class="line"> <span class="comment">//...</span></div><div class="line">}</div><div class="line"></div><div class="line"><span class="keywordflow">if</span>((ptr = <span class="keyword">nullptr</span>) || (cond = <span class="keyword">false</span>)) <span class="comment">// Trust me, I know what I'm doing. No warning.</span></div><div class="line">{</div><div class="line"> <span class="comment">//...</span></div><div class="line">}</div></div><!-- fragment --><h2><a class="anchor" id="S5_1_1_rules"></a> |
| Rules</h2> |
| <ul> |
| <li>Use spaces for indentation and alignment. No tabs! Indentation should be done with 4 spaces.</li> |
| <li>Unix line returns in all the files.</li> |
| <li>Pointers and reference symbols attached to the variable name, not the type (i.e. char &foo;, and not char& foo).</li> |
| <li>No trailing spaces or tabs at the end of lines.</li> |
| <li>No spaces or tabs on empty lines.</li> |
| <li>Put { and } on a new line and increase the indentation level for code inside the scope (except for namespaces).</li> |
| <li>Single space before and after comparison operators ==, <, >, !=.</li> |
| <li>No space around parenthesis.</li> |
| <li>No space before, one space after ; (unless it is at the end of a line).</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keywordflow">for</span>(<span class="keywordtype">int</span> i = 0; i < width * height; ++i)</div><div class="line">{</div><div class="line"> <span class="keywordtype">void</span> *d = foo(ptr, i, &addr);</div><div class="line"> static_cast<uint8_t *>(data)[i] = static_cast<uint8_t *>(d)[0];</div><div class="line">}</div></div><!-- fragment --><ul> |
| <li>Put a comment after #else, #endif, and namespace closing brace indicating the related name</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keyword">namespace </span>mali</div><div class="line">{</div><div class="line"><span class="preprocessor">#ifdef MALI_DEBUG</span></div><div class="line"> ...</div><div class="line">#<span class="keywordflow">else</span> <span class="comment">// MALI_DEBUG</span></div><div class="line"> ...</div><div class="line">#endif <span class="comment">// MALI_DEBUG</span></div><div class="line">} <span class="comment">// namespace mali</span></div></div><!-- fragment --><ul> |
| <li>CamelCase for class names only and lower case words separated with _ (snake_case) for all the functions / methods / variables / arguments / attributes.</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keyword">class </span>ClassName</div><div class="line">{</div><div class="line"> <span class="keyword">public</span>:</div><div class="line"> <span class="keywordtype">void</span> my_function();</div><div class="line"> <span class="keywordtype">int</span> my_attribute() <span class="keyword">const</span>; <span class="comment">// Accessor = attribute name minus '_', const if it's a simple type</span></div><div class="line"> <span class="keyword">private</span>:</div><div class="line"> <span class="keywordtype">int</span> _my_attribute; <span class="comment">// '_' in front of name</span></div><div class="line">};</div></div><!-- fragment --><ul> |
| <li>Use quotes instead of angular brackets to include local headers. Use angular brackets for system headers.</li> |
| <li>Also include the module header first, then local headers, and lastly system headers. All groups should be separated by a blank line and sorted lexicographically within each group.</li> |
| <li>Where applicable the C++ version of system headers has to be included, e.g. cstddef instead of stddef.h.</li> |
| <li>See <a href="http://llvm.org/docs/CodingStandards.html#include-style">http://llvm.org/docs/CodingStandards.html#include-style</a></li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="preprocessor">#include "MyClass.h"</span></div><div class="line"></div><div class="line"><span class="preprocessor">#include "arm_cv/core/Helpers.h"</span></div><div class="line"><span class="preprocessor">#include "arm_cv/core/Types.h"</span></div><div class="line"></div><div class="line"><span class="preprocessor">#include <cstddef></span></div><div class="line"><span class="preprocessor">#include <numeric></span></div></div><!-- fragment --><ul> |
| <li>Only use "auto" when the type can be explicitly deduced from the assignment.</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keyword">auto</span> a = static_cast<float*>(bar); <span class="comment">// OK: there is an explicit cast</span></div><div class="line"><span class="keyword">auto</span> <a class="code" href="namespacearm__compute_1_1test_1_1validation.xhtml#aa76b4a6e74940dabc5b7fc6b2dab3545">b</a> = std::make_unique<Image>(foo); <span class="comment">// OK: we can see it's going to be an std::unique_ptr<Image></span></div><div class="line"><span class="keyword">auto</span> c = img.ptr(); <span class="comment">// NO: Can't tell what the type is without knowing the API.</span></div><div class="line"><span class="keyword">auto</span> d = vdup_n_u8(0); <span class="comment">// NO: It's not obvious what type this function returns.</span></div></div><!-- fragment --><ul> |
| <li>OpenCL:<ul> |
| <li>Use __ in front of the memory types qualifiers and kernel: __kernel, __constant, __private, __global, __local.</li> |
| <li>Indicate how the global workgroup size / offset / local workgroup size are being calculated.</li> |
| <li>Doxygen:<ul> |
| <li>No '*' in front of argument names</li> |
| <li>[in], [out] or [in,out] <em>in front</em> of arguments</li> |
| <li>Skip a line between the description and params and between params and <dl class="section return"><dt>Returns</dt><dd>(If there is a return)</dd></dl> |
| </li> |
| <li>Align params names and params descriptions (Using spaces), and with a single space between the widest column and the next one.</li> |
| <li>Use an upper case at the beginning of the description</li> |
| </ul> |
| </li> |
| </ul> |
| </li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="comment"> /** Set the input and output tensor.</span></div><div class="line"><span class="comment"> *</span></div><div class="line"><span class="comment"> * @note If the output tensor is a nullptr or is equal to the input, the activation function will be performed in-place</span></div><div class="line"><span class="comment"> *</span></div><div class="line"><span class="comment"> * @param[in, out] input Source tensor. In case of @p output tensor = nullptr, this tensor will store the result</span></div><div class="line"><span class="comment"> * of the activation function. Data types supported: QASYMM8/QASYMM8_SIGNED/QSYMM16/F16/F32.</span></div><div class="line"><span class="comment"> * @param[out] output Destination tensor. Data type supported: same as @p input</span></div><div class="line"><span class="comment"> * @param[in] activation_info Activation layer parameters.</span></div><div class="line"><span class="comment"> */</span></div><div class="line"> <span class="keywordtype">void</span> configure(ITensor *<a class="code" href="namespacearm__compute_1_1test_1_1validation.xhtml#a8fcf2ddd9a1d58b1b280f5c0aed71845">input</a>, ITensor *output, ActivationLayerInfo activation_info);</div></div><!-- fragment --> <h2><a class="anchor" id="S5_1_2_how_to_check_the_rules"></a> |
| How to check the rules</h2> |
| <p>astyle (<a href="http://astyle.sourceforge.net/">http://astyle.sourceforge.net/</a>) and clang-format (<a href="https://clang.llvm.org/docs/ClangFormat.html">https://clang.llvm.org/docs/ClangFormat.html</a>) can check and help you apply some of these rules.</p> |
| <h2><a class="anchor" id="S5_1_3_library_size_guidelines"></a> |
| Library size: best practices and guidelines</h2> |
| <h3><a class="anchor" id="S5_1_3_1_template_suggestions"></a> |
| Template suggestions</h3> |
| <p>When writing a new patch we should also have in mind the effect it will have in the final library size. We can try some of the following things:</p> |
| <ul> |
| <li>Place non-dependent template code in a different non-templated class/method</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keyword">template</span><<span class="keyword">typename</span> T></div><div class="line"><span class="keyword">class </span>Foo</div><div class="line">{</div><div class="line"><span class="keyword">public</span>:</div><div class="line"> <span class="keyword">enum</span> { v1, v2 };</div><div class="line"> <span class="comment">// ...</span></div><div class="line">};</div></div><!-- fragment --> <pre class="fragment">can be converted to: |
| </pre><div class="fragment"><div class="line"><span class="keyword">struct </span>Foo_base</div><div class="line">{</div><div class="line"> <span class="keyword">enum</span> { v1, v2 };</div><div class="line"> <span class="comment">// ...</span></div><div class="line">};</div><div class="line"></div><div class="line"><span class="keyword">template</span><<span class="keyword">typename</span> T></div><div class="line"><span class="keyword">class </span>Foo : <span class="keyword">public</span> Foo_base</div><div class="line">{</div><div class="line"><span class="keyword">public</span>:</div><div class="line"> <span class="comment">// ...</span></div><div class="line">};</div></div><!-- fragment --><ul> |
| <li>In some cases it's preferable to use runtime switches instead of template parameters</li> |
| <li>Sometimes we can rewrite the code without templates and without any (significant) performance loss. Let's say that we've written a function where the only use of the templated argument is used for casting:</li> |
| </ul> |
| <div class="fragment"><div class="line"><span class="keyword">template</span> <<span class="keyword">typename</span> T></div><div class="line"><span class="keywordtype">void</span> <a class="code" href="namespacearm__compute_1_1test_1_1validation.xhtml#a06403ad5596b5405787cfca12e5b815e">NETemplatedKernel::run</a>(<span class="keyword">const</span> Window &window)</div><div class="line">{</div><div class="line">...</div><div class="line"> *(reinterpret_cast<T *>(out.ptr())) = *(reinterpret_cast<const T *>(in.ptr()));</div><div class="line">...</div><div class="line">}</div></div><!-- fragment --><p>The above snippet can be transformed to:</p> |
| <div class="fragment"><div class="line"><span class="keywordtype">void</span> <a class="code" href="namespacearm__compute_1_1test_1_1validation.xhtml#a06403ad5596b5405787cfca12e5b815e">NENonTemplatedKernel::run</a>(<span class="keyword">const</span> Window &window)</div><div class="line">{</div><div class="line">...</div><div class="line">std::memcpy(out.ptr(), in.ptr(), element_size);</div><div class="line">...</div><div class="line">}</div></div><!-- fragment --><h2><a class="anchor" id="S5_1_4_secure_coding_practices"></a> |
| Secure coding practices</h2> |
| <h3><a class="anchor" id="S5_1_4_1_general_coding_practices"></a> |
| General Coding Practices</h3> |
| <ul> |
| <li><b>Use tested and approved managed code</b> rather than creating new unmanaged code for common tasks.</li> |
| <li><b>Utilize locking to prevent multiple simultaneous requests</b> or use a synchronization mechanism to prevent race conditions.</li> |
| <li><b>Protect shared variables and resources</b> from inappropriate concurrent access.</li> |
| <li><b>Explicitly initialize all your variables and other data stores</b>, either during declaration or just before the first usage.</li> |
| <li><b>In cases where the application must run with elevated privileges, raise privileges as late as possible, and drop them as soon as possible</b>.</li> |
| <li><b>Avoid calculation errors</b> by understanding your programming language's underlying representation and how it interacts with numeric calculation. Pay close attention to byte size discrepancies, precision, signed/unsigned distinctions, truncation, conversion and casting between types, "not-a-number" calculations, and how your language handles numbers that are too large or too small for its underlying representation.</li> |
| <li><b>Restrict users from generating new code</b> or altering existing code.</li> |
| </ul> |
| <h3><a class="anchor" id="S5_1_4_2_secure_coding_best_practices"></a> |
| Secure Coding Best Practices</h3> |
| <ul> |
| <li><b>Validate input</b>. Validate input from all untrusted data sources. Proper input validation can eliminate the vast majority of software vulnerabilities. Be suspicious of most external data sources, including command line arguments, network interfaces, environmental variables, and user controlled files.</li> |
| <li><b>Heed compiler warnings</b>. Compile code using the default compiler flags that exist in the SConstruct file.</li> |
| <li>Use <b>static analysis tools</b> to detect and eliminate additional security flaws.</li> |
| <li><b>Keep it simple</b>. Keep the design as simple and small as possible. Complex designs increase the likelihood that errors will be made in their implementation, configuration, and use. Additionally, the effort required to achieve an appropriate level of assurance increases dramatically as security mechanisms become more complex.</li> |
| <li><b>Default deny</b>. Base access decisions on permission rather than exclusion. This means that, by default, access is denied and the protection scheme identifies conditions under which access is permitted</li> |
| <li><b>Adhere to the principle of least privilege</b>. Every process should execute with the least set of privileges necessary to complete the job. Any elevated permission should only be accessed for the least amount of time required to complete the privileged task. This approach reduces the opportunities an attacker has to execute arbitrary code with elevated privileges.</li> |
| <li><b>Sanitize data sent to other systems</b>. Sanitize all data passed to complex subsystems such as command shells, relational databases, and commercial off-the-shelf (COTS) components. Attackers may be able to invoke unused functionality in these components through the use of various injection attacks. This is not necessarily an input validation problem because the complex subsystem being invoked does not understand the context in which the call is made. Because the calling process understands the context, it is responsible for sanitizing the data before invoking the subsystem.</li> |
| <li><b>Practice defense in depth</b>. Manage risk with multiple defensive strategies, so that if one layer of defense turns out to be inadequate, another layer of defense can prevent a security flaw from becoming an exploitable vulnerability and/or limit the consequences of a successful exploit. For example, combining secure programming techniques with secure runtime environments should reduce the likelihood that vulnerabilities remaining in the code at deployment time can be exploited in the operational environment.</li> |
| </ul> |
| <h1><a class="anchor" id="S5_2_how_to_submit_a_patch"></a> |
| How to submit a patch</h1> |
| <p>To be able to submit a patch to our development repository you need to have a GitHub account. With that, you will be able to sign in to Gerrit where your patch will be reviewed.</p> |
| <p>Next step is to clone the Compute Library repository: </p><pre class="fragment">git clone "ssh://<your-github-id>@review.mlplatform.org:29418/ml/ComputeLibrary" |
| </pre><p>If you have cloned from GitHub or through HTTP, make sure you add a new git remote using SSH: </p><pre class="fragment">git remote add acl-gerrit "ssh://<your-github-id>@review.mlplatform.org:29418/ml/ComputeLibrary" |
| </pre><p>After that, you will need to upload an SSH key to <a href="https://review.mlplatform.org/#/settings/ssh-keys">https://review.mlplatform.org/#/settings/ssh-keys</a></p> |
| <p>Then, make sure to install the commit-msg Git hook in order to add a change-ID to the commit message of your patch: </p><pre class="fragment">cd "ComputeLibrary" && mkdir -p .git/hooks && curl -Lo `git rev-parse --git-dir`/hooks/commit-msg https://review.mlplatform.org/tools/hooks/commit-msg; chmod +x `git rev-parse --git-dir`/hooks/commit-msg) |
| </pre><p>When your patch is ready, remember to sign off your contribution by adding a line with your name and e-mail address to every git commit message: </p><pre class="fragment">Signed-off-by: John Doe <john.doe@example.org> |
| </pre><p>You must use your real name, no pseudonyms or anonymous contributions are accepted.</p> |
| <p>You can add this to your patch with: </p><pre class="fragment">git commit -s --amend |
| </pre><p>You are now ready to submit your patch for review: </p><pre class="fragment">git push acl-gerrit HEAD:refs/for/master |
| </pre><h1><a class="anchor" id="S5_3_code_review"></a> |
| Patch acceptance and code review</h1> |
| <p>Once a patch is uploaded for review, there is a pre-commit test that runs on a Jenkins server for continuos integration tests. In order to be merged a patch needs to:</p> |
| <ul> |
| <li>get a "+1 Verified" from the pre-commit job</li> |
| <li>get a "+1 Comments-Addressed", in case of comments from reviewers the committer has to address them all. A comment is considered addressed when the first line of the reply contains the word "Done"</li> |
| <li>get a "+2" from a reviewer, that means the patch has the final approval</li> |
| </ul> |
| <p>At the moment, the Jenkins server is not publicly accessible and for security reasons patches submitted by non-whitelisted committers do not trigger the pre-commit tests. For this reason, one of the maintainers has to manually trigger the job.</p> |
| <p>If the pre-commit test fails, the Jenkins job will post a comment on Gerrit with the details about the failure so that the committer will be able to reproduce the error and fix the issue, if any (sometimes there can be infrastructure issues, a test platform disconnecting for example, where the job needs to be retriggered). </p> |
| </div></div><!-- PageDoc --> |
| </div><!-- contents --> |
| </div><!-- doc-content --> |
| <!-- start footer part --> |
| <div id="nav-path" class="navpath"><!-- id is needed for treeview function! --> |
| <ul> |
| <li class="footer">Generated on Thu Mar 5 2020 16:07:08 for Compute Library by |
| <a href="http://www.doxygen.org/index.html"> |
| <img class="footer" src="doxygen.png" alt="doxygen"/></a> 1.8.15 </li> |
| </ul> |
| </div> |
| </body> |
| </html> |
