summaryrefslogtreecommitdiff
path: root/doc
diff options
context:
space:
mode:
authorSamuel Ortiz <sameo@linux.intel.com>2012-07-18 12:23:49 +0200
committerSamuel Ortiz <sameo@linux.intel.com>2012-07-18 12:23:49 +0200
commit7f28ebe69c36354245224e43daf01b32d997c5b9 (patch)
treed283b3b82ec9ba28434fa7611d6761cb4ce88e51 /doc
parentc28f8256ca105da2d3b1f47a3a348fa47e7d89c6 (diff)
downloadconnman-7f28ebe69c36354245224e43daf01b32d997c5b9.tar.gz
connman-7f28ebe69c36354245224e43daf01b32d997c5b9.tar.bz2
connman-7f28ebe69c36354245224e43daf01b32d997c5b9.zip
doc: Add coding-style.txt
This is a copy of the oFono coding-style document, with oFono instances replaced with ConnMan ones. It's also in sync with the neard one.
Diffstat (limited to 'doc')
-rw-r--r--doc/coding-style.txt344
1 files changed, 344 insertions, 0 deletions
diff --git a/doc/coding-style.txt b/doc/coding-style.txt
new file mode 100644
index 00000000..30690b76
--- /dev/null
+++ b/doc/coding-style.txt
@@ -0,0 +1,344 @@
+Every project has its coding style, and ConnMan is not an exception. This
+document describes the preferred coding style for ConnMan 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, ConnMan 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)". ConnMan 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, ConnMan 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
+ connman_dbus_dict_append();
+
+2)
+if (call->status == CALL_STATUS_ACTIVE ||
+ call->status == CALL_STATUS_HELD) { // correct
+ connman_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 ConnMan
+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);