mirror of
https://git.kernel.org/pub/scm/network/wireless/iwd.git
synced 2024-12-25 08:02:38 +01:00
doc: Add coding-style.txt from oFono
This commit is contained in:
parent
61245bc14e
commit
54b0b7d9e9
345
doc/coding-style.txt
Normal file
345
doc/coding-style.txt
Normal file
@ -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);
|
Loading…
Reference in New Issue
Block a user