Version 15 (modified by 7 years ago) (diff) | ,
---|
This coding style is not final and not yet in effect. Discussion ongoing in #openvpn-devel. Right before branching of the release/2.4 branch, we will have The Great Reformatting where we switch to this coding style.
Introduction
The OpenVPN coding style is based on the Allman style, e.g.:
void myfunction(int a, int b) { const char *dummy = "Hello world!"; if (a == b) { something_equals(dummy); } else { does_not_equal(dummy); } }
Summarizing:
- Indentation is 4 spaces, no tabs ever.
- Opening and closing brackets get their own line, and must match indentation.
- Line length is 80 characters.
- C99 is allowed. E.g.
for (int i = 0; i < max; i++)
. - Only use
/* */
-style comments
Line wrapping
The maximum line length is 80 characters. When lines exceed this length, wrap them using a double indent (ie 8 spaces) on the new line. For example:
void my_function(void) { if (variable_with_artificially_long_name_1 != 0 && variable_with_artificially_long_name_2 != 0) { return variable_with_artificially_long_name_1 + variable_with_artificially_long_name_2 + variable_with_artificially_long_name_3; } }
If you find yourself continuously exceeding the line limit, you might want to consider breaking up your function into smaller functions to reduce nesting.
Some additional secure coding style rules
This are style rules, not general secure coding guide lines. See e.g. https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard for useful guidelines on writing secure C.
All branches should use braces
For example:
if (a) { return true; }
We wouldn't be the first to not spot missing braces in an if statement, like what happened in Apple's goto fail; bug. Always using braces prevents this kind of mistake.
Cases in a switch statement should break or explicitly fall through
The only exception to this rule are empty case statements, to not overly clutter the code with comments.
For example:
switch (a) { case ONE: case TWO: one_or_two = true; /* Fall through */ case THREE: now_do_something(); break; default: ASSERT(0); }
This makes the intent immediately clear to the reader / reviewer.
Crustify rules
The most fit tool for our purpose (because it can be told to just leave certain constructs alone, while changing others) seems to be
uncrustify -c config tun.c
... but it has a zillion of options (600)... our config-in-the-making:
# Use Allman-style, space-only indent_columns=4 indent_with_tabs=0 align_with_tabs=false indent_braces=false indent_else_if=false nl_if_brace=add nl_brace_else=add nl_elseif_brace=add nl_else_brace=add nl_else_if=remove sp_func_proto_paren=Remove sp_func_def_paren=Remove sp_func_call_paren=Remove sp_sizeof_paren=Remove # Do not put spaces between the # and preprocessor statements pp_space=remove cmt_convert_tab_to_spaces=true indent_switch_case=4 indent_label=1 sp_before_sparen=add sp_inside_sparen=remove sp_cond_colon=add sp_cond_question=add sp_bool=add sp_else_brace=add sp_brace_else=add pos_arith=Lead nl_func_type_name=add nl_before_case=true nl_assign_leave_one_liners=true nl_enum_leave_one_liners=true nl_brace_fparen=add nl_max=4 nl_after_func_proto=2 code_width=80 ls_code_width=false mod_full_brace_if=add mod_full_brace_if_chain=false mod_add_long_ifdef_endif_comment=20 mod_add_long_ifdef_else_comment=5 mod_remove_extra_semicolon=true cmt_c_nl_end=true cmt_star_cont=true # Use "char **a"-style pointer stars sp_before_ptr_star=Add sp_between_ptr_star=Remove sp_after_ptr_star=Remove
(For reference: ident and astyle have been tried, but had either limitations or bugs that made them unusable for us.)