summaryrefslogtreecommitdiffstats
path: root/doc/coding-style.txt
blob: 287e9e9280bab51ab94d736f6c3d400e3f2f1262 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
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);