1 Every project has its coding style, and ConnMan is not an exception. This
2 document describes the preferred coding style for ConnMan code, in order to keep
3 some level of consistency among developers so that code can be easily
4 understood and maintained, and also to help your code survive under
5 maintainer's fastidious eyes so that you can get a passport for your patch
8 First of all, ConnMan coding style must follow every rule for Linux kernel
9 (http://www.kernel.org/doc/Documentation/CodingStyle). There also exists a tool
10 named checkpatch.pl to help you check the compliance with it. Just type
11 "checkpatch.pl --no-tree patch_name" to check your patch. In theory, you need
12 to clean up all the warnings and errors except this one: "ERROR: Missing
13 Signed-off-by: line(s)". ConnMan does not used Signed-Off lines, so including
14 them is actually an error. In certain circumstances one can ignore the 80
15 character per line limit. This is generally only allowed if the alternative
16 would make the code even less readable.
18 Besides the kernel coding style above, ConnMan has special flavors for its own.
19 Some of them are mandatory (marked as 'M'), while some others are optional
20 (marked as 'O'), but generally preferred.
22 M1: Blank line before and after an if/while/do/for statement
23 ============================================================
24 There should be a blank line before if statement unless the if is nested and
25 not preceded by an expression or variable declaration.
52 The only exception to this rule applies when a variable is being allocated:
53 array = g_try_new0(int, 20);
54 if (!array) // Correct
58 M2: Multiple line comment
59 =========================
60 If your comments have more then one line, please start it from the second line.
64 * first line comment // correct
70 M3: Space before and after operator
71 ===================================
72 There should be a space before and after each operator.
80 If your condition in if, while, for statement or a function declaration is too
81 long to fit in one line, the new line needs to be indented not aligned with the
86 if (call->status == CALL_STATUS_ACTIVE ||
87 call->status == CALL_STATUS_HELD) { // wrong
88 connman_dbus_dict_append();
91 if (call->status == CALL_STATUS_ACTIVE ||
92 call->status == CALL_STATUS_HELD) { // correct
93 connman_dbus_dict_append();
96 gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len,
97 enum sim_ust_service index) // wrong
104 gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len,
105 enum sim_ust_service index) // correct
111 If the line being wrapped is a function call or function declaration, the
112 preferred style is to indent at least past the opening parenthesis. Indenting
113 further is acceptable as well (as long as you don't hit the 80 character
116 If this is not possible due to hitting the 80 character limit, then indenting
117 as far as possible to the right without hitting the limit is preferred.
122 gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len,
123 enum sim_ust_service index); // worse
126 gboolean sim_ust_is_available(unsigned char *service_ust, unsigned char len,
127 enum sim_ust_service index);
130 M5: Git commit message 50/72 formatting
131 =======================================
132 The commit message header should be within 50 characters. And if you have
133 detailed explanatory text, wrap it to 72 character.
136 M6: Space when doing type casting
137 =================================
138 There should be a space between new type and variable.
142 a = (int *)b; // wrong
144 a = (int *) b; // correct
147 M7: Don't initialize variable unnecessarily
148 ===========================================
149 When declaring a variable, try not to initialize it unless necessary.
154 for (i = 0; i < 3; i++) {
158 M8: Abort if small allocation fail
159 ==================================
160 When g_malloc fails, the whole program would exit. Small allocations
161 are very unlikely to fail and if an allocations is not possible, it is
162 very likely the error code can't recover from this
163 situation. Furthermore, many of the error paths are not tested at
164 all. Instead use g_malloc() when the allocation fails and rely on an
165 external watchdog to restart the program.
167 Furthermore, Glib's functions such as g_strdup use g_malloc which
168 obviously terminates the program anyway.
170 For large allocation using g_try_malloc is still the right choice.
173 additional = g_try_malloc(len - 1); // correct
178 M9: Follow the order of include header elements
179 ===============================================
180 When writing an include header the various elements should be in the following
183 - forward declarations
187 - function declarations and inline function definitions
190 M10: Internal headers must not use include guards
191 =================================================
192 Any time when creating a new header file with non-public API, that header
193 must not contain include guards.
199 Enums must have a descriptive name. The enum type should be small caps and
200 it should not be typedef-ed. Enum contents should be in CAPITAL letters and
201 prefixed by the enum type name.
206 ANIMAL_TYPE_FOUR_LEGS,
207 ANIMAL_TYPE_EIGHT_LEGS,
208 ANIMAL_TYPE_TWO_LEGS,
211 If the enum contents have values (e.g. from specification) the formatting
212 should be as follows:
215 ANIMAL_TYPE_FOUR_LEGS = 4,
216 ANIMAL_TYPE_EIGHT_LEGS = 8,
217 ANIMAL_TYPE_TWO_LEGS = 2,
220 M12: Enum as switch variable
223 If the variable of a switch is an enum, you must not include a default in
224 switch body. The reason for this is: If later on you modify the enum by adding
225 a new type, and forget to change the switch accordingly, the compiler will
226 complain the new added type hasn't been handled.
231 ANIMAL_TYPE_FOUR_LEGS = 4,
232 ANIMAL_TYPE_EIGHT_LEGS = 8,
233 ANIMAL_TYPE_TWO_LEGS = 2,
239 case ANIMAL_TYPE_FOUR_LEGS:
242 case ANIMAL_TYPE_EIGHT_LEGS:
245 case ANIMAL_TYPE_TWO_LEGS:
252 However if the enum comes from an external header file outside ConnMan
253 we cannot make any assumption of how the enum is defined and this
254 rule might not apply.
256 M13: Check for pointer being NULL
257 =================================
259 When checking if a pointer or a return value is NULL, use the
260 check with "!" operator.
264 array = g_try_new0(int, 20);
265 if (!array) // Correct
269 if (!g_at_chat_get_slave(chat)) // Correct
273 array = g_try_new0(int, 20);
274 if (array == NULL) // Wrong
278 M14: Always use parenthesis with sizeof
279 =======================================
280 The expression argument to the sizeof operator should always be in
285 memset(stuff, 0, sizeof(*stuff));
288 memset(stuff, 0, sizeof *stuff); // Wrong
291 M15: Use void if function has no parameters
292 ===========================================================
293 A function with no parameters must use void in the parameter list.
306 M16: Don't use hex value with shift operators
307 ==============================================
308 The expression argument to the shift operators should not be in hex.
320 Better to use abbreviation, rather than full name, to name a variable,
321 function, struct, etc.
324 supplementary_service // too long
327 O2: Try to avoid complex if body
328 ================================
329 It's better not to have a complicated statement for if. You may judge its
330 contrary condition and return | break | continue | goto ASAP.
336 call = synthesize_outgoing_call(vc, vc->pending);
337 v = voicecall_create(vc, call);
338 v->detect_time = time(NULL);
339 DBG("Registering new call: %d", call->id);
340 voicecall_dbus_register(v);
349 call = synthesize_outgoing_call(vc, vc->pending);
350 v = voicecall_create(vc, call);
351 v->detect_time = time(NULL);
352 DBG("Registering new call: %d", call->id);
353 voicecall_dbus_register(v);