not logged in | [Login]

The goal is to have consistent, clear code. All code should share a common style, format, choice of function / variable names, etc. This means that people looking at the code see the code, and not the wildly different coding styles.

Code should be commented. There are over 100K LoC in the server, and it's impossible to always remember why things are done the way they are. The comments should explain the choices behind the code, the goal, or philosophy, or the "gotcha's".

Function documentation should be doxygen style. Below is an example of doxygen function documentation, use it as a guide when writing your own.

/** Simple wrapper to decide whether an IP value is v4 or v6 and call the appropriate parser
 *
 * @param out Where to write the parsed ip address.
 * @param value to parse.
 * @param inlen Length of value, if value is \0 terminated inlen may be -1.
 * @param resolve If true and value doesn't look like an IP address, try and resolve value as
 *	a hostname.
 * @return
 *	- 0 if ip address was parsed successfully.
 *	- -1 on failure.
 */
int fr_pton(fr_ipaddr_t *out, char const *value, ssize_t inlen, bool resolve)
{
}

Function naming conventions

Verb / noun order

Nouns should always precede verbs <prefix>_<noun>_<verb>[_<verb qualifiers>].

Good

fr_request_alloc()
fr_dict_find_by_name()
python_interp_exec()

Bad

do_python()
rest_parse_pairs()

Name component separation

Name components should be separated by underscores.

Good

fr_pair_find

Bad

frPairFind
frpairfind
fr_pairfind

Prefixes / namespaces

Protocol library fr_

All functions in the protocol library should be prefixed with fr. The rationale is that as libfreeradius is a library intended to be used by 3rd party applications it requires a namespace to avoid conflicts.

Server library <group>_

Functions in the server library should not have a global prefix, but should use a common prefix i.e. functions which manipulate value pairs should use the pair_ prefix, dictionary functions should use the dict_ prefix.

Modules

Public functions should use the mod_ prefix, e.g. mod_authorize, mod_authentication, mod_instantiate. If the module uses its own library, those functions should be prefixed with the name of the module e.g. python_, rest_, eap_.

Ideally, all of the functions in a module should be static. If they cannot be that, the function name prefixes given above are required.

Instantiation and destruction

Library and structure initialisations functions should use the verb init. Functions that initialise structures should expect them to contain garbage values (be uninitialised).

Structure allocation functions should use the verb alloc. create should not be used.

Explicit free or destroy functions should be avoided, and talloc destructors used wherever possible. Talloc destructor functions should be static and be prefixed with _ to mark them as private e.g. _fr_pair_free()

Callbacks

Callbacks should be prefixed with a _ to mark them as private e.g. int _my_comparator(int a, int b).

Return types

Boolean

bool is used where the function is answering a true/false question (is a predicate). A good example is the is_whitespace function, which checks if the entire string is made up of whitespace.

bool is_whitespace(char const *value)
{
	do {
		if (!isspace(*value)) return false;
	} while (*++value);

	return true;
}

Integer

int should be used for most functions. Negative integers indicate failure, zero indicates success, positive integers indicate success with caveats.

If you find a function returns significant (>= 4) numbers of different integer values, you should define C preprocessor macros, and an enum type so that it's clear in calling code what the different values mean.

typedef enum {
	FUNC_RCODE_SUCCESS = 0,
	FUNC_RCODE_NO_MEMORY = -1,
	FUNC_RCODE_BAD_ARGUMENTS_0 = -2,
	FUNC_RCODE_BAD_ARGUMENTS_1 = -3
} func_rcode_t;

func_rcode_t my_function(int arg0, int arg1)
{
	void *mem;

	mem = talloc(NULL, void);
	if (!mem) return FUNC_RCODE_NO_MEMORY;

	if (arg0 > 10) return FUNC_RCODE_BAD_ARGUMENTS_0;
	if (arg1 > 10) return FUNC_RCODE_BAD_ARGUMENTS_1;

	return FUNC_RCODE_SUCCESS;
}

Pointer

Used only for allocation functions where there's a single simple failure mode (in which case NULL is returned).

my_structure_t *simple_memory_alloc(TALLOC_CTX *ctx, int arg0, int arg1)
{
	return talloc(ctx, my_structure_t);
}

For more complicated failures the function should return an integer, and write a pointer to the allocated memory via one of the arguments.

int complicated_memory_alloc(TALLOC_CTX *ctx, my_structure_t **out, int arg0, int arg1)
{
	my_structure_t *structure.

	if (arg0 == 0) {
		fr_strerror_printf("Invalid parameter, arg0 must be greater than 0");
		return -2;
	}	

	structure = talloc(ctx, my_structure_t);
	if (!structure) return -1;

	return 0;
}

Argument order

Arguments should be in the following order

  • TALLOC_CTX (if used).
  • Arguments that return data to the caller ([out] arguments in doxygen parlence).
  • Arguments for passing data to the function.

For modules the instance data pointer should be directly before the REQUEST * pointer.

Code style

FreeRADIUS code style is very similar to K&R/Linux Kernel style https://www.kernel.org/doc/Documentation/CodingStyle.

Linux Kernel style should be used as the base coding style (excluding chapters 5,9,11,13,20), with the rules described below taking precedence.

No trailing whitespace, newline at the end of the file

Many editors support trimming trailing whitespace and adding a newline at the end of the file automatically.

You should enable these features.

Arguments align with the end of the function name

This is true for function declarations, definitions, and calls. Grouping related arguments together on the same line is also preferred.

Good

int my_function_with_lots_of_args(fr_ipaddr_t *src, uint16_t src_port,
				  fr_ipaddr_t *dst, uint16_t dst_port,
				  struct timeval timeout)
{
	return 0;
}

Bad

int my_function_with_lots_of_args(fr_ipaddr_t *src, uint16_t src_port, fr_ipaddr_t *dst, uint16_t dst_port,
	struct timeval timeout)
{
	return 0;
}

Hard break at 120 columns

It's not 1978, 80x24 VT100 terminals have long been consigned to the electronics recycling bin (hopefully).

We believe code is perfectly readable up to 120 columns, that and over application of hard line breaking at 80 columns reduces code readability instead of increasing it.

Strings should also be broken at 120 columns for consistency.

Always collapse bodies (where possible)

If a condition or loop and its body can be collapsed onto a single line, then do so.

Good

if (foo && bar) baz();
while (foo()) baz();

Bad

if (foo && bar) {
	baz();
}
if (foo && bar)
	baz();
while (foo()) {
	baz();
}
while (foo())
	baz();

Return early

Avoid unnecessary condition nesting by returning as soon as there's an error, or the function has done its useful work.

Gotos are a perfectly acceptable way to reduce the number of exit points. You'll often see the finish or release label used in module code for this purpose.

Good

int my_func(char const *value)
{
	void *out;

	if (input_parse(&out, value) < 0) return -1;
	if (output_write(out) < 0) return -1;

	return 0;
}

Bad

int my_func(char const *value)
{
	void *out;

	if (input_parse(&out, value) == 0) {
		if (output_write(out) < 0) return -1;
	} else {
		return -1;
	}

	return 0;
}

Error gotos jump backwards

Error exit points and their goto labels should be defined the first time they occur in a function.

Good

int my_func(char const *value)
{
	void *out;

	if (input_parse(&out, value) < 0) {
	error:
		ERROR("Something bad happened");
		return -1;
	}
	if (output_write(out) < 0) goto error;

	return 0;
}

Bad

int my_func(char const *value)
{
	void *out;

	if (input_parse(&out, value) < 0) goto error;
	if (output_write(out) < 0) goto error;

	return 0;

error:
	ERROR("Something bad happened");
	return -1;
}

Const on the right

Allows const to be read consistently as applying to the type on the left.

Good

char const * const *

Bad

const char * const *

Variable declarations at the top of blocks

Variables don't have to be defined at the start of the function, but they should be defined at the start of a block. This can be any type of block, even an anonymous one {}.

Grouping declarations helps make the code more readable, and makes it easier to see the variables in use within certain areas of the code.

Good

int func(int a, int b)
{
	char *my_func_var;
	
	if (a) {
		char *my_if_var;
		
		if (b) return -1;
		my_if_var = "bar";
	}

	my_func_var = "foo";
	return 0;
}

Bad

inf func(int a, int b)
{
	if (a) {
		if (b) return 0;
		char *my_if_var = "bar";
	}

	char *my_func_var = "foo";
	return 0;
}