c can be a nightmare - the problems are well documented. Here's some notes on my methodology that help make code clearer, easier to maintain and less likely to contain memory leaks.
Every routine should have a single return point. All possible execution paths go through this point and therefore have the chance to have resources released. The main thing, of course, is to free() memory that has been malloc()'ed.
The evils of 'goto' were all too well propagated when "structured programming" got started in the 70-80's. But it still has a vital role in simplifying code.
The best place for the return logic block is at the end of the routine under a label. When errors are detected and the routine needs to bail, a simple "goto end" (or "goto hell" if you prefer) allows cleanup to be done. This also eliminates tortuous if-else sphaghetti code. Think of it as exception processing.
Do it even when it looks stupidly simple - when the routine is made more elaborate then the logic is in place and you will remember to cleanup:
/*
* @return 1 if filename is executable. Otherwise 0.
*/
int is_executable(
char *filename, /**< filename IN name of file to test */
int verbose /**< verbose IN set to non-0 to make it verbose */
) {
struct stat buf;
int retval = 0;
if (verbose) printf("Checking %s\n", filename);
if (stat(filename, &buf)) {
goto end;
}
if ((buf.st_mode & S_IFMT) == S_IFREG) {
if (buf.st_mode & 0111) {
retval = 1;
goto end;
}
}
end:
return(retval);
}
Especially pointers which should be initialised to NULL. Then, the return logic can clean them up if non-NULL.
Unless you are absolutely sure there's no risk of buffer overflow. Definitely if the source string comes from a user.
The same applies to using snprintf() rather than sprintf().
Ideally, avoid the libc str* functions entirely and create your own safe library using something like:
typedef struct {
int len;
char *str;
} SAFE_STRING;
This also has the benefit of potentially being more efficient of CPU cycles as it avoids the constant and repeated use of strlen(). Think about it.
If at all possible, always return an int to indicate success or failure eg. 0 for success, anything else for an error. Or return a pointer on success and NULL on failure.
When there is an error condition and the routine is expected to return malloc'd strings, then that string should be free'd - don't leave it to the caller to free them unless the routine returns success. Callers of your routine are somewhat likely to test for the error condition but much less likely to free up pointers too!
Just do it. No, the source code is not enough documentation. And keep it up to date as you code. You might as well use doxygen format in case the employer cultivates a taste for it.
Unless you've got a really good excuse, the program should exit(0) on success and non-zero on failure (note that exit code can only be up to 8-bits long).
As with shell programming, the program should send error messages to stderr, output to stdout. It should recognise -h (no matter what) and possibly --help and print help to stdout (and return 0!). -v and --verbose are also pretty standard.
In fact, why not use argp(3) to make option processing easy?
You're not done until valgrind says you're done!!