[Freeipa-devel] Suggested additions to codding standard page on freeIPA wiki.

[Dmitri Pal]

Hello,

In addition to the coding guidelines published on freeIPA.org the 
following (currently undefined) rules are suggested:

*Spaces:*

No tabs all indentation 4 spaces.

*Length of line: *

Not more than 100-110 characters. Just keep it reasonable. Longer lines 
are harder to deal with due to different sized of monitors and different 
editors used.

*Module structure: *

The module should be structured in the following order:
* Copyright boilerplate
* Includes
* Local defines – used only inside module. Global/shared defines should 
be in the header
* Local module typedefs - Global/shared typedefs should be in the header
* Module variable declarations – variables that are global for the module.
* Declarations of external functions
* Declarations of static functions
* Implementation of the exposed functions from the module
* Implementation of the static functions

It is not required but recommended to group and position functions in 
the module so that it is easier to understand.

*Comments: *

Sections (groups of functions) can be separated with a line of comments 
like this:

/************ Main Interface Functions ****************/

Or this

/********* Static Internal Functions **********/

Each function must have a comment describing what this function does and 
returns.
There is no specific format for this but it is a good practice to 
describe what parameters mean and what return codes function returns 
under what conditions.

Inside the function comments should help reader to digest the logic of 
the function. They should answer the main question “why is this line of 
code here and what it tries to accomplish?”. Not everything should be 
commented but practice shows that comments giving the hint about what 
the developer what trying to accomplish with this or that construct 
usually turn out to be pretty helpful.

*Naming of the variables: *


There are multiple different styles that developers usually use to name 
variables. It can be very short like “ctx” or “”buf”; more explicit 
“context” or “buffer” ; detailed using underscore “hash_context” or 
“input_buffer”; detailed using capital letters “hashContext” or 
“inputBuffer” all these ways of naming variables are acceptable though 
low case with underscore is preferable and highly recommended. The 
developers should not start variables with capital letter. It is 
beneficial and thus suggested to use self explanatory names for 
variables that carry significant information. Variables that are used to 
perform a simple operation and act as a temporary storage can have 
shorter names.
Do not use Hungarian notation (prefixing variable with the type). The 
only exception to the rule is pointers. The following example 
illustrates the case:

void function_that_does_something(data_set inventory) {
int number_items =0;

err=get_number_of_items_in_set(inventory, number_items);
...
}

int get_number_of_items_in_set(data_set inventory, int* p_number_items) {
...
}

In this case the variable in the second function is related to the 
variable in the calling function but it is a pointer. This is useful 
since naming variables same will cause confusion with type (especially 
if there will be cutting and pasting of code which I use to do a lot), 
and naming them differently will break the relation. Like having 
number_items in one function and item_count in the other and one is 
actually a pointer to another is hard to work with. Prefixing with "p_" 
solves the issue nicely.

*Names of the functions: *

Looking at the code (server.c for example) I have seen all sorts of 
names for functions used in one module:

setup_signals(void)
BlockSignals(true, SIGPIPE);
poptGetNextOpt(pc)

We should try to stick to one style which will be low case, multi word, 
_ separated like:
monitor_task_init(struct task_server *task);

* Declarations *
Major declarations such as typedef's, stuct tags, etc. deserve to be 
identifiable on their own in some fashion.
Often this was done with an initial cap, but since we're using 
lower_case_with_underscores then I think the convention of appending 
"_t" is a good way to go and is consistent with the Posix and kernel 
conventions.


Other name formats should not be used unless it is an external function 
we do not have control over.

The wiki page will be updated based on this discussion.

Thanks
Dmitri