-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Description
Every now and then when contributing code we come across situations where reviewers ask for changes according to coding style that has become common practice within OpenSSL but for some reason is not (yet) reflected in the official published reference document https://www.openssl.org/policies/codingstyle.html https://www.openssl.org/policies/technical/coding-style.html.
The contents of this document apparently have not been updated since Jan 12 2015.
It should be extended to contain things like:
Rules on expressions handled in openssl/technical-policies#46:
- Do not place binary logical operators
&&and||at the end of a line but at the beginning of the following line. Theyshouldmay get an extra indentation (of +4 characters). - Make (in-)equality checks with integer
0(but not with Boolean 'false') explicit, e.g., writeif (i != 0)rather thanif (i)
and(x & MASK) == 0rather than!(x & MASK). - In comparisons with constants (including
NULL) the constant should be on the right-hand side of the comparison operator. - Avoid needless parentheses, e.g., replace
if ((ptr == NULL) && (!f(x)))byif (ptr == NULL && !f(x)). - Put parentheses around uses of macro arguments (unless they are passed on as-is to a further macro or function).
- Put parentheses around macro bodies that can be used as expressions.
Rules on whitespace and comments handled in openssl/technical-policies#47:
- Avoid empty lines at the beginning or at the end of a file.
- Avoid multiple empty lines in a row.
- Have an empty line between local variable declarations and subsequent code.
- Put a space after commas and after semicolons in
forstatements, but not infor(;;) - Do not use multiple consecutive spaces except for indentation and for multi-line alignments of definitions, e.g.,
#define FOO_INVALID -1 /* invalid or inconsistent arguments */
#define FOO_INTERNAL 0 /* Internal error, most likely malloc */
#define FOO_OK 1 /* success */
#define FOO_GREAT 100 /* some specific outcome */
- Place comments above or to the right of the code they refer to.
- Comments referring to the code line
before orafter should be indented equally to that code line. If a line barely exceeds the 80-char width limit and adding a line break would clearly reduce readability, the line does not need to be split.- Make sure that comments do not contain spelling errors.
Rules on names handled in openssl/technical-policies#48:
Use brief but descriptive names for variables, types, functions, etc.- Avoid single-letter names except for loop indices such as
iandj, in particular when they can be visually confusing, such asIandO. - For getter and setter functions use names containing
get0_orget1_(rather thanget_) andset0_orset1_(rather thanset_) andpush0_orpush1_(rather thanpush_) to point out whether the value parameter has aliasing or copying semantics. - Align to RFC-defined terms and wording
including camelCase where appropriate. - Use lowercase prefix like
ossl_for internal symbols. - Use uppercase prefix like
EVP_orOSSL_CMP_for public (API) symbols. - Make sure that names do not contain spelling errors.
Rules on error handling:
- Check for any potential error condition, in particular those potentially reported in the return value of (non-void) functions called.
- Avoid needless checks, for instance
ptr == NULLbefore calling ..._free(ptr). - Be consistent with error reporting on a per-function basis: when adding errors to the OpenSSL error stack (using
ERR_raise()) then this should happen in all error paths. When some other function is called then assume this already has been achieved by that function, where a comment may be added such as "XYZerr already called" to make it clear that the omission of a specific error is deliberate. - Avoid
goto errwhen areturnstatement is sufficient. - Functions returning essentially a Boolean indication of success (
1) or failure (0) of typeintmay return
-1for something bad (e.g., invalid argument, internal error, or memory allocation failure)
-2to indicate an unsupported feature
etc. (TODO: are there more such conventions?)
Misc stuff:
- Do not place statements after the '
:' of aswitchcaseordefault. - Do not place
#definebetween code lines in function bodies - Do not use
#ifwith constant condition, such asif 0, nor other forms of unconditionally disabling or commenting out code. - Do not use the comma operator between statement-like expressions where otherwise a compound statement (using
{and}) would be needed, e.g.,
if (condition)
print_error(), exit(1);
- Avoid calling ...
_dup()functions when ..._up_ref()is sufficient, e.g., forX509. - Use
constfor types wherever possible (but avoid needlessconstfor scalar types in function prototypes) - Do not use
enumin public headers (i.e., ininclude/openssl/). - Use existing OpenSSL macros for declarations as far as possible, e.g., for function prototypes and type definitions.
- Document all members of the public API (i.e., in
include/openssl/), including functions, macros, and types indoc/man3/. Optionally document internal functions etc. indoc/internal/man3/. - Document all new/updated features that affect the CLI (i.e.,
apps/openssl) user experience indoc/man1/(at a suitable level of abstraction).