diff --git a/doc/coding-style.txt b/doc/coding-style.txt new file mode 100644 index 00000000..287e9e92 --- /dev/null +++ b/doc/coding-style.txt @@ -0,0 +1,345 @@ +Every project has its coding style, and oFono is not an exception. This +document describes the preferred coding style for oFono 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, oFono coding style must follow every rule for Linux kernel +(http://www.kernel.org/doc/Documentation/CodingStyle). There also exists a tool +named checkpatch.pl to help you check the compliance with it. Just type +"checkpatch.pl --no-tree patch_name" to check your patch. In theory, you need +to clean up all the warnings and errors except this one: "ERROR: Missing +Signed-off-by: line(s)". oFono does not used Signed-Off lines, so including +them is actually an error. 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, oFono 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 == NULL) // 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, explicitly compare to +NULL rather than use the shorter check with "!" operator. + +Example: +1) +array = g_try_new0(int, 20); +if (!array) // Wrong + return; + +2) +if (!g_at_chat_get_slave(chat)) // Wrong + return -EINVAL; + +3) +array = g_try_new0(int, 20); +if (array == NULL) // Correct + 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 + +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);