Coding Methods

If you don’t, you’ll be forced to debug it six months later, when you have no clue as to what it’s doing.

If someone really hates you, you’ll be forced to debug un-commented code that someone else wrote. You don’t want to do that.

For FreeRADIUS, use doxygen @-style comments so you get the benefits of the developer documentation site, https://doc.freeradius.org/.

Variables and functions should have names. Calling them x, xx, and xxx makes your life hell. Even foo and i are problematic.

Avoid smurfs. Don’t reuse struct names in field names, i. e.

If your code reads as full English sentences, you’re doing it right.

Your function cannot do anything right if the user passed in garbage and you were too lazy to check for garbage input.

assert() (fr_assert()) is ugly. Use it.

"Function failed" is useless as an error message. It makes debugging the code impossible without source-level instrumentation.

If you’re going to instrument the code at source level for error messages, leave the error messages there, so the next sucker won’t have to do the same work all over again.

Your function cannot do anything right if you called another function, and they gave you garbage output.

One of the most common mistakes is:

If the programmer had bothered to check for a NULL fp (error condition), then they could have produced a descriptive error message instead of having the program core dump.

If your program core dumps accidentally, you’re a bad programmer. You don’t know what your program is doing, or what it’s supposed to be doing when anything goes wrong.

If it hits an assert() and calls abort(), you’re a genius. You’ve thought ahead to what might go wrong, and put in an assertion to ensure that it fails in a known manner when something does go wrong. (As it usually does…)

memset() (talloc_zero()) is your friend. ptr = NULL is nice, too.

Having variables containing garbage values makes it easy for the code to do garbage things. The contents of local variables are inputs to your function. See #3.

It’s also nearly impossible for you to debug any problems, as you can’t tell the variables with garbage values from the real ones.

They’re usually accidental, but they cause core dumps. strcpy() and strcat() are ugly. Use them under duress.

sizeof() is your friend.

If you don’t mean to modify an input structure to your function, declare it const. Declare string constants const. It can’t hurt, and it allows more errors to be found at compile time.

Use const everywhere. Once you throw a few into your code, and have it save you from stupid bugs, you’ll blindly throw in const everywhere. It’s a life-saver.

Turn on all of the C compiler warnings possible. You might have to turn some off due to broken system header files, though. But the more warnings the merrier.

Getting error messages at compile time is much preferable to getting core dumps at run time. See #7.

Notice that the C compiler error messages are helpful? You should write error messages like this, too. See #4.

You don’t know under what system someone will try to run your code. Don’t demand that others use the same OS or character set as you use.

Never assign numbers to pointers. If foo is a char*, and you want it to be be NULL, assign NULL, not 0. The zeroth location is perfectly as addressable as any other on plenty of OSes. Not all the world runs on Unix (though it should :) ).

Another common mistake is to assume that the zeroth character in the character set is the string terminator. Instead of terminating a string with 0, use '\0', which is always right. Similarly, memset() with the appropriate value: NULL, '\0', or 0 for pointers, chars, and numbers.

Don’t put tabs in string constants, either. Always use '\t' to represent a tab, instead of ASCII 9. Literal tabs are presented to readers of your code as arbitrary whitespace, and it’s easy to mess up.

Though it’s legal to test if (foo) {} if you test against the appropriate value (like NULL or '\0'), your code is prettier and easier for others to read without having to eyeball your prototypes continuously to figure out what you’re doing (especially if your variables aren’t well-named). See #2.

Even Donald Knuth writes buggy code. You’ll never find all of the bugs in your code. But writing a test program definitely helps.

This means that you’ll have to write your code so that it will be easily testable. As a result, it will look better and be easier to debug.

This section lists many of the common rules associated with code submitted to the project. There are always exceptions… but you must have a really good reason for doing so.

The FreeRADIUS server has a common coding style. Use real tabs to indent. There is whitespace in variable assignments (i = 1, not i=1).

When in doubt, format your code to look the same as code already in the server. If your code deviates too much from the current style, it is likely to be rejected without further review, and without comment.

Code cluttered with #ifdef s is difficult to read and maintain. Don’t do it. Instead, put your #ifdef s in a header, and conditionally define static inline functions, or macros, which are used in the code. Let the compiler optimize away the 'no-op' case.

Simple example, of poor code:

Cleaned-up example:

(in header):

(in the code itself):

Static inline functions are greatly preferred over macros. They provide type safety, have no length limitations, no formatting limitations, and under gcc they are as cheap as macros.

Macros should only be used for cases where a static inline is clearly suboptimal (there a few, isolated cases of this in fast paths), or where it is impossible to use a static inline function (such as string-izing).

static inline is preferred over static __inline__, extern inline, and extern __inline__.

Don’t try to anticipate nebulous future cases which may or may not be useful: Make it as simple as you can, and no simpler.

Split up functionality as much as possible. If your code needs to do two unrelated things, write two functions. Mashing two kinds of work into one function makes the server difficult to debug and maintain.