mirror of
https://git.kernel.org/pub/scm/network/wireless/iwd.git
synced 2024-11-22 23:09:34 +01:00
9e11175cf8
To add forward-declaration of statics rule. This rule is already enforced, but for some reason the document in iwd did not have this rule in it (it is in other projects, like oFono)
358 lines
8.3 KiB
Plaintext
358 lines
8.3 KiB
Plaintext
Background
|
|
==========
|
|
|
|
Every project has its coding style, and the Wireless daemon is not an
|
|
exception. This document describes the preferred coding style for the
|
|
Wireless daemon 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, the Wireless daemon 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 --no-signoff patch_name" to
|
|
check your patch.
|
|
|
|
In theory, you need to clean up all the warnings and errors. 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, the Wireless daemon 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) // 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, use the shorter check
|
|
with "!" operator rather than explicitly compare to NULL.
|
|
|
|
Example:
|
|
1)
|
|
array = g_try_new0(int, 20);
|
|
if (!array) // Correct
|
|
return;
|
|
|
|
2)
|
|
array = g_try_new0(int, 20);
|
|
if (array == NULL) // Wrong
|
|
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
|
|
|
|
|
|
M17: Avoid forward-declaration of static functions
|
|
==================================================
|
|
|
|
Functions that are static should not be forward-declared. The only exception
|
|
to this rule is if a circular dependency condition exists, and the forward
|
|
declaration cannot be avoided.
|
|
|
|
|
|
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);
|