Linux kernel

I just stumbled on this piece of code and wonder if this is common practice or just negligence. If name is NULL then it will crash. So why is there no check?
1
2
3
4
5
6
7
static void push_string(const char *name)
{
  unsigned int name_len = strlen(name) + 1;
  fputs(name, stdout);
  putchar(0);
  offset += name_len;
}

https://github.com/torvalds/linux/blob/master/usr/gen_init_cpio.c
Last edited on
It appears that the checking is being done in the function that calls this function, if this is always the case then checking for NULL here would be a redundant check.

https://en.wikipedia.org/wiki/Bounds_checking
It's really a design question: At what point in the call stack or logic should bounds checking be done? For example, the philosophy in the C library is to never do bounds checking (for speed), and put the responsibility on the user of the function (e.g. the strlen call in your example). Parts of C++'s standard library are more forgiving (std::stoi creates an exception if no conversion could be performed, but an invalid idx pointer still causes undefined behavior).

I don't think there's a good general solution to "should I check the bounds of the input parameter here?".
It all depends on how it's called.

I like to think about examples IN SPACE: If a spacecraft is a millsecond away from impact to land and its speed is still too fast... well, we can watch out for that in code and throw an exception or return and log an error message if we detect it, but that spacecraft is still going to crash. A function earlier in the process should have prevented it from happening in the first place; it's too late now except for debugging purposes (assert), and you better believe you're going to do debugging after crashing your $125 million spacecraft.
Last edited on
> So why is there no check?

One reason is that the function has internal linkage; all the direct calls to the function are from within the same translation unit. Even so, ideally it should have been documented:
static void push_string(const char *name) // invariant: name is not NULL

Another reason (important when writing libraries) is: if required, it is easy to add a safer, but slower wrapper, to an underlying implementation that is fast. It is not possible, without breaking code, to make a slow underlying implementation faster.

The general philosophy in C and C++ has been: do not penalise programmers who write correct code because there may be others who write incorrect code. For instance, in
for( std::size_t i = 0 ; i < vec.size() ; ++i ) { /* access the char with vec[i] */ }
it would be counterproductive to perform range checks on each application of the subscript operator.
Often, both a fast unchecked version, and a slower checked version may be provided. eg. vec.at(i)
It appears that the checking is being done in the function that calls this function, if this is always the case then checking for NULL here would be a redundant check.

Maybe that's the idea behind but it looks still a bit risky. If the caller doesn't check....

Another reason (important when writing libraries) is: if required, it is easy to add a safer, but slower wrapper, to an underlying implementation that is fast. It is not possible, without breaking code, to make a slow underlying implementation faster.

Why not just using assert? Debug builds are always slower so an additional check wouldn't hurt.
safer wrappers are used in production as well, not just in debug builds. Putting the check in a wrapper allows you to use the core function directly (and quickly) in cases where you know the input is not null. Also, everything you do in kernel land is going to be risky.
Is there any (cheap) check that would make this code safer?
One of only two things can happen. Either name is null or it isn't. If it's null, strlen() will cause a panic, same as an assert() would have. If it's not null strlen() might crash anyway, if the pointer is invalid, but an assert() would not be able to detect it.

So without any added information, I don't see what would be the point of adding a check in this function. Best case scenario, you might end up masking a bug in a function higher in the stack.
If it's null, strlen() will cause a panic, same as an assert() would have.
assert will give you a message, file and line number.
Not sure what the standard says about the nullptr, maybe undefined behavior?
Against an invalid ptr probably only a memory manager would help.
You can obtain the location of a null dereference in kernel from a memory dump. The precise nature of the crash (an invalid read) and the memory location involved will appear in the dump as well. So I don't see much value being added by an assert().
Topic archived. No new replies allowed.