Should I check every single parameter of a function to make sure the function works well?

Question

I notice that the standard c library contains several string functions that don't check the input parameter(whether it's NULL), like strcmp:

int strcmp(const char *s1, const char *s2)
{
    for ( ; *s1 == *s2; s1++, s2++)
    if (*s1 == '\0')
        return 0;
    return ((*(unsigned char *)s1 < *(unsigned char *)s2) ? -1 : +1);
}

And many others do not do the same validation. Is this a good practice?

In other library, I saw they check every single parameter, like this:

int create_something(int interval, int mode, func_t cb, void *arg, int id)
{
    if (interval == 0) return err_code_1;
    if (valid(mode))   return err_code_2;
    if (cb == NULL)    return err_code_3;
    if (arg == NULL)   return err_code_4;
    if (id == 0)       return err_code_5;

    // ...
}

Which one is better? When you design an API, would you check all parameters to make it function well or just let it go crash?

Answer 1

I'd like to argue that not checking pointers for NULL in library functions that expect valid pointers is actually better practice than to do error returns or silently ignoring them.

NULL is not the only invalid pointer. There are billions of other pointer values that are actually incorrect, why should we give preferential treatment to just one value?

Error returns are often ignored, misunderstood or mismanaged. Forgetting to check one error return could lead to a misbehaving program. I'd like to argue that a program that silently misbehaves is worse than a program that doesn't work at all. Incorrect results can be worse than no results.

Failing early and hard eases debugging. This is the biggest reason. An end user of a program doesn't want the program to crash, but as a programmer I'm the end user of a library and I actually want it to crash. Crashing makes it evident that there's a bug I need to fix and the faster we hit the bug and the closer the crash is to the source of the bug, the faster and easier I can find it and fix it. A NULL pointer dereference is one of the most trivial bugs to catch, debug and fix. It's much easier than trawling through gigabytes of logs to spot one line that says "create_something had a null pointer".

With error returns, what if the caller catches that error, returns an error itself (in your example that would be err_create_something_failed) and its caller returns another error (err_caller_of_create_something_failed)? Then you have an error return 3 functions away, that might not even indicate what actually went wrong. And even if it manages to indicate what actually went wrong (by having a whole framework for error handling that records exactly where the error happened through the whole chain of callers) the only thing you can do with it is to look up the error value in some table and from that conclude that there was a NULL pointer in create_something. It's a lot of pain when instead you could just have opened a debugger and seen exactly where the assumption was violated and what exact chain of function calls lead to that problem.

In the same spirit you can use assert to validate other function arguments to cause early and easy to debug failures. Crash on the assert and you have the full correct call chain that leads to the problem. I just wouldn't use asserts to check pointers because it's pointless (at least on an operating system with memory management) and makes things slower while giving you the same behavior (minus the printed message).