Skip to content

The coding style guideline document should be updated #10725

@DDvO

Description

@DDvO

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. They should may get an extra indentation (of +4 characters).
  • Make (in-)equality checks with integer 0 (but not with Boolean 'false') explicit, e.g., write if (i != 0) rather than if (i)
    and (x & MASK) == 0 rather 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))) by if (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 for statements, but not in for(;;)
  • 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 or after 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 i and j, in particular when they can be visually confusing, such as I and O.
  • For getter and setter functions use names containing get0_ or get1_ (rather than get_) and set0_ or set1_ (rather than set_) and push0_ or push1_ (rather than push_) 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_ or OSSL_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 == NULL before 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 err when a return statement is sufficient.
  • Functions returning essentially a Boolean indication of success (1) or failure (0) of type int may return
    -1 for something bad (e.g., invalid argument, internal error, or memory allocation failure)
    -2 to indicate an unsupported feature
    etc. (TODO: are there more such conventions?)

Misc stuff:

  • Do not place statements after the ':' of a switch case or default.
  • Do not place #define between code lines in function bodies
  • Do not use #if with constant condition, such as if 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., for X509.
  • Use const for types wherever possible (but avoid needless const for scalar types in function prototypes)
  • Do not use enum in public headers (i.e., in include/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 in doc/man3/. Optionally document internal functions etc. in doc/internal/man3/.
  • Document all new/updated features that affect the CLI (i.e., apps/openssl) user experience in doc/man1/ (at a suitable level of abstraction).

Metadata

Metadata

Assignees

No one assigned

    Labels

    backlog fixThe issue was closed as part of the backlog reduction initiative.triaged: documentationThe issue/pr deals with documentation (errors)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions