| Background |
| ========== |
| |
| Every project has its coding style, and the Wireless daemon is not an |
| exception. This document describes the preferred coding style for the |
| Wireless daemon code, in order to keep some level of consistency among |
| developers so that code can be easily understood and maintained, and also |
| to help your code survive under maintainer's fastidious eyes so that you |
| can get a passport for your patch ASAP. |
| |
| First of all, the Wireless daemon coding style must follow every rule for |
| Linux kernel (http://www.kernel.org/doc/Documentation/process/coding-style.rst). |
| There also exists a tool named checkpatch.pl to help you check the compliance |
| with it. Just type "checkpatch.pl --no-tree --no-signoff patch_name" to |
| check your patch. |
| |
| In theory, you need to clean up all the warnings and errors. In certain |
| circumstances one can ignore the 80 character per line limit. This is |
| generally only allowed if the alternative would make the code even less |
| readable. |
| |
| Besides the kernel coding style above, the Wireless daemon has special |
| flavors for its own. Some of them are mandatory (marked as 'M'), while |
| some others are optional (marked as 'O'), but generally preferred. |
| |
| |
| M1: Blank line before and after an if/while/do/for statement |
| ============================================================ |
| There should be a blank line before if statement unless the if is nested and |
| not preceded by an expression or variable declaration. |
| |
| Example: |
| 1) |
| a = 1; |
| if (b) { // wrong |
| |
| 2) |
| a = 1 |
| |
| if (b) { |
| } |
| a = 2; // wrong |
| |
| 3) |
| if (a) { |
| if (b) // correct |
| |
| 4) |
| b = 2; |
| |
| if (a) { // correct |
| |
| } |
| |
| b = 3; |
| |
| The only exception to this rule applies when a variable is being allocated: |
| array = g_try_new0(int, 20); |
| if (array) // Correct |
| return; |
| |
| |
| M2: Multiple line comment |
| ========================= |
| If your comments have more then one line, please start it from the second line. |
| |
| Example: |
| /* |
| * first line comment // correct |
| * ... |
| * last line comment |
| */ |
| |
| |
| M3: Space before and after operator |
| =================================== |
| There should be a space before and after each operator. |
| |
| Example: |
| a + b; // correct |
| |
| |
| M4: Wrap long lines |
| =================== |
| If your condition in if, while, for statement or a function declaration is too |
| long to fit in one line, the new line needs to be indented not aligned with the |
| body. |
| |
| Example: |
| 1) |
| if (call->status == CALL_STATUS_ACTIVE || |
| call->status == CALL_STATUS_HELD) { // wrong |
| ofono_dbus_dict_append(); |
| |
| 2) |
| if (call->status == CALL_STATUS_ACTIVE || |
| call->status == CALL_STATUS_HELD) { // correct |
| ofono_dbus_dict_append(); |
| |
| 3) |
| gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len, |
| num sim_ust_service index) // wrong |
| { |
| int a; |
| ... |
| } |
| |
| 4) |
| gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len, |
| enum sim_ust_service index) // correct |
| { |
| int a; |
| ... |
| } |
| |
| If the line being wrapped is a function call or function declaration, the |
| preferred style is to indent at least past the opening parenthesis. Indenting |
| further is acceptable as well (as long as you don't hit the 80 character |
| limit). |
| |
| If this is not possible due to hitting the 80 character limit, then indenting |
| as far as possible to the right without hitting the limit is preferred. |
| |
| Example: |
| |
| 1) |
| gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len, |
| enum sim_ust_service index); // worse |
| |
| 2) |
| gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len, |
| enum sim_ust_service index); |
| // better |
| |
| |
| M5: Git commit message 50/72 formatting |
| ======================================= |
| The commit message header should be within 50 characters. And if you have |
| detailed explanatory text, wrap it to 72 character. |
| |
| |
| M6: Space when doing type casting |
| ================================= |
| There should be a space between new type and variable. |
| |
| Example: |
| 1) |
| a = (int *)b; // wrong |
| 2) |
| a = (int *) b; // correct |
| |
| |
| M7: Don't initialize variable unnecessarily |
| =========================================== |
| When declaring a variable, try not to initialize it unless necessary. |
| |
| Example: |
| int i = 1; // wrong |
| |
| for (i = 0; i < 3; i++) { |
| } |
| |
| |
| M8: Use g_try_malloc instead of g_malloc |
| ======================================== |
| When g_malloc fails, the whole program would exit. Most of time, this is not |
| the expected behavior, and you may want to use g_try_malloc instead. |
| |
| Example: |
| additional = g_try_malloc(len - 1); // correct |
| if (additional == NULL) |
| return FALSE; |
| |
| |
| M9: Follow the order of include header elements |
| =============================================== |
| When writing an include header the various elements should be in the following |
| order: |
| - #includes |
| - forward declarations |
| - #defines |
| - enums |
| - typedefs |
| - function declarations and inline function definitions |
| |
| |
| M10: Internal headers must not use include guards |
| ================================================= |
| Any time when creating a new header file with non-public API, that header |
| must not contain include guards. |
| |
| |
| M11: Naming of enums |
| ==================== |
| Enums must have a descriptive name. The enum type should be small caps and |
| it should not be typedef-ed. Enum contents should be in CAPITAL letters and |
| prefixed by the enum type name. |
| |
| Example: |
| |
| enum animal_type { |
| ANIMAL_TYPE_FOUR_LEGS, |
| ANIMAL_TYPE_EIGHT_LEGS, |
| ANIMAL_TYPE_TWO_LEGS, |
| }; |
| |
| If the enum contents have values (e.g. from specification) the formatting |
| should be as follows: |
| |
| enum animal_type { |
| ANIMAL_TYPE_FOUR_LEGS = 4, |
| ANIMAL_TYPE_EIGHT_LEGS = 8, |
| ANIMAL_TYPE_TWO_LEGS = 2, |
| }; |
| |
| |
| M12: Enum as switch variable |
| ==================== |
| |
| If the variable of a switch is an enum, you must not include a default in |
| switch body. The reason for this is: If later on you modify the enum by adding |
| a new type, and forget to change the switch accordingly, the compiler will |
| complain the new added type hasn't been handled. |
| |
| Example: |
| |
| enum animal_type { |
| ANIMAL_TYPE_FOUR_LEGS = 4, |
| ANIMAL_TYPE_EIGHT_LEGS = 8, |
| ANIMAL_TYPE_TWO_LEGS = 2, |
| }; |
| |
| enum animal_type t; |
| |
| switch (t) { |
| case ANIMAL_TYPE_FOUR_LEGS: |
| ... |
| break; |
| case ANIMAL_TYPE_EIGHT_LEGS: |
| ... |
| break; |
| case ANIMAL_TYPE_TWO_LEGS: |
| ... |
| break; |
| default: // wrong |
| break; |
| } |
| |
| However if the enum comes from an external header file outside ofono |
| we cannot make any assumption of how the enum is defined and this |
| rule might not apply. |
| |
| |
| M13: Check for pointer being NULL |
| ================================= |
| When checking if a pointer or a return value is NULL, use the shorter check |
| with "!" operator rather than explicitly compare to NULL. |
| |
| Example: |
| 1) |
| array = g_try_new0(int, 20); |
| if (!array) // Correct |
| return; |
| |
| 2) |
| array = g_try_new0(int, 20); |
| if (array == NULL) // Wrong |
| return; |
| |
| |
| M14: Always use parenthesis with sizeof |
| ======================================= |
| The expression argument to the sizeof operator should always be in |
| parenthesis, too. |
| |
| Example: |
| 1) |
| memset(stuff, 0, sizeof(*stuff)); |
| |
| 2) |
| memset(stuff, 0, sizeof *stuff); // Wrong |
| |
| |
| M15: Use void if function has no parameters |
| =========================================================== |
| A function with no parameters must use void in the parameter list. |
| |
| Example: |
| 1) |
| void foo(void) |
| { |
| } |
| |
| 2) |
| void foo() // Wrong |
| { |
| } |
| |
| |
| M16: Don't use hex value with shift operators |
| ============================================== |
| The expression argument to the shift operators should not be in hex. |
| |
| Example: |
| |
| 1) |
| 1 << y |
| |
| 2) |
| 0x1 << y // Wrong |
| |
| |
| M17: Avoid forward-declaration of static functions |
| ================================================== |
| |
| Functions that are static should not be forward-declared. The only exception |
| to this rule is if a circular dependency condition exists, and the forward |
| declaration cannot be avoided. |
| |
| |
| O1: Shorten the name |
| ==================== |
| Better to use abbreviation, rather than full name, to name a variable, |
| function, struct, etc. |
| |
| Example: |
| supplementary_service // too long |
| ss // better |
| |
| |
| O2: Try to avoid complex if body |
| ================================ |
| It's better not to have a complicated statement for if. You may judge its |
| contrary condition and return | break | continue | goto ASAP. |
| |
| Example: |
| 1) |
| if (a) { // worse |
| struct voicecall *v; |
| call = synthesize_outgoing_call(vc, vc->pending); |
| v = voicecall_create(vc, call); |
| v->detect_time = time(NULL); |
| DBG("Registering new call: %d", call->id); |
| voicecall_dbus_register(v); |
| } else |
| return; |
| |
| 2) |
| if (!a) |
| return; |
| |
| struct voicecall *v; |
| call = synthesize_outgoing_call(vc, vc->pending); |
| v = voicecall_create(vc, call); |
| v->detect_time = time(NULL); |
| DBG("Registering new call: %d", call->id); |
| voicecall_dbus_register(v); |