the Chromium logo

The Chromium Projects

Native Client Coding Style Guidelines

The purpose of this document is to codify our existing coding style guidelines for new Native Client project members, including external committers. Some of the recommendations address security concerns, e.g., making the code easier to audit. A core principle is that our coding style should encourage writing correct code. Moreover, it should encourage writing obviously correct, easily audited code.

Generally, we follow the published Google Coding Style guidelines. However, those guidelines were written primarily for C++ and python code, and a portion of NaCl code is in C (esp kernel-like code); furthermore, we use scons/gyp/subversion rather than gconfig/perforce, so build system issues are also addressed. Additionally, we incorporate/use bodies of third party code or their interfaces, most obvious of which is NPAPI, and stylistic differences occur, especially among major modules. In such cases, it is generally a good idea to adhere to the existing style -- for example, the Google Coding Style expressly does not endorse one position of the space in declarations such as char *p; versus char* p; over another -- and in such cases maintaining consistency with the prevalent style in the module would enhance readability.

The Google C++ coding style guide can be found here:; the python version can be found here:

C/C++/Build Style Rules

C Style Rules

/* num_elt is untrusted */

if (num_elt > SIZE_T_MAX / sizeof *ptr) {

/* error handling / abort */


ptr = malloc(num_elt * sizeof *ptr);

if (NULL == ptr) {

/* error handling */


similarly, even assignments can cause problems:

int32_t len = strlen(some_c_str); /* on 64-bit systems, a problem also with int64_t */

here, since strlen returns a value of type size_t, it may be a value that cannot be represented as an int32_t -- according to the C (and C++) standard, when such an assignment occurs and the expression value is not representable in the destination type, the result is implementation dependent. And when this occurs, compilers are free to generate code that delete all of the users' files (though we know of no such compiler!). Instead, check:

size_t len_sz = strlen(some_c_str);

if (len_sz > INT32_MAX) { /* from stdint.h */

/* error handling */


len = (int32_t) len_sz; /* len_sz unused henceforth, so this is just renaming a register / memory location */

to ensure that overflows won't cause problems. Similarly, even additions can overflow:

int32_t foo; int32_t bar; ... foo = bar + 1;

should be

int32_t foo;

int32_t bar;


if (bar > INT32_MAX - 1) {

/* error handling */


foo = bar + 1;

*   See the malloc idiom above. The amount of memory to be allocated
            should always use a dereference of the left-hand-side of the
            assignment, since the lhs must be of the right type. Rationale:
            if for some reason we later change the type of a variable, there
            is no need to track down occurrences of sizeof(Typename)
            elsewhere, since the use of the lhs in the sizeof will make it
            automatically correct.

Comparisons involving a constant should keep the constant on the lhs of the comparison operator. This avoids accidents like

if (ptr = NULL) {

/* oops */


where a comparison gets turned into an assignment (followed by implicit comparison to 0) due to a typographical error, changing the meaning of the code. Instead, use the idom from the larger code snippet above. Since the lhs is not an lvalue, this cannot be accidentally turned into an assignment. While not a strict requirement, constants should be used on the lhs even for inequality tests, since in the future somebody else might change things around and turn it (or via cut-n-paste) into an intended equality test.

Once we have a test to ensure that we do not accidentally turn off compiler-specific flags that warn when constructs like if (lvalue = expression) is used in code, we can relax this rule.

*   Rather than refering to a distantly defined value/variable such
            as a constant defined in a header or the top of the file

#define BUFFER_SIZE 1000

/* ... */

char buffer[BUFFER_SIZE];

/* ... */

memset(buffer, 0, BUFFER_SIZE); /* bad */

instead, do

memset(buffer, 0, sizeof buffer); /* better (Yes, the parentheses are not required around buffer here. It surprised cbiffle and sehr too.) */

The latter is more obviously correct. (But not if buffer is actually a char * to malloc'd memory, so auditors still have to do some looking around.)

*   add more lines...

C++ Style Rules

bool SanityCheckerApi(SomeType obj, char* data, int32_t nbytes);

bool SomeClass::SomeMethod(std::string s) {

if (SanityCheckerApi(obj_, s.c_str(), static_cast<int32_t>(s.size()))) {

// Compilers correctly warn that precision is lost without a cast,

// since std::string::size() is size_t, and on a 64-bit machine is

// large than int32_t. However, just using a static_cast<int32_t> to

// get rid of the compiler warning messages is wrong.

return false; // error


/* Use all of s */

return true;


can cause a security problem: if the caller-supplied data s is under the adversary's control and could be more than 4GB in size, then SanityCheckerApi will only check the first s.size() mod 2**32 bytes of data, and the use of s after the check will (presumably) consume all of s and do the wrong thing.