return 0 is better than return ret

There are a lot of people who write “return ret;” when they mean “return 0;” I feel like I’m the person who cares about this the most in the world, but hopefully after reading this blog you will notice it as well and it will annoy you.

Most functions in the Linux kernel return zero on success and return negative error codes on failure. There are some functions which return negative error codes or a positive number on success. I have previously written that callers should check “if (ret) ” for functions which return zero and only use “if (ret < 0)” for functions which return positive values. But other people disagree. The sound subsystem in particular. Whatever. But if you’re going to always check for negatives please return a literal zero instead of “return ret;”

Consider this code:

ret = frob();
if (ret < 0)
        return ret;

frob();
frob();
frob();

return ret;

It fills you with rage, right? If it had been written as “return 0;” it would have been immediately obvious. But the way it’s written, it could be any non-negative number like three or one hundred. Who knows? Now you have to scroll back to see the “ret = frob();” assignment and then you have to jump into the frob() function to see what that returns. Normally when you jump into frob(), it’s pretty obvious that it returns zero or negative error codes. But imagine if that function followed the same anti-pattern and you had to jump further up the call tree. Imagine it took you hours and hours to walk the call tree back until eventually you got so enraged you grabbed your computer monitor and smashed it on the floor. Imagine that! WOW! You need to talk to someone about your anger issues!

Quite often the code is obvious but annoying:

ret = frob();
if (ret) {
        printf("error: frob failed\n");
        return ret;
}

return ret;

This code works as intended but it still is less readable than “return 0;”.

I am an amateur snake enthusiast but there are some people who make their career around snakes. When the professionals go into the forest they spot things which the rest of us don’t see. They’ve trained their eyes and they understand the habitat. When the rest of see a snake, we see a green snake but a professional can identify the species from just a glance. “Did you see how dark its eyes are? It’s a Western Natal Green Snake.”

It’s the same for kernel developers. We look at code all day long. Things like a return 0; statement stand out to us and we’re subconsciously looking for it.

There is one time where return ret is especially annoying and that is this:

ret = short_cut();
if (!ret)
        return ret;
frob();
frob();

What this code is saying is that if the short_cut() succeeds then we can return directly, but otherwise we have to do it the long way. But to the eye the short cut looks like an error path.

if (!ret)
123456789
        return ret;
0123456789012345678

The code has 38 characters in it and 37 of them are saying “this is an error path” and one character is saying this is a success path. I review it every time someone adds code like this to the kernel because sometimes the “!” character was added unintentionally. Normally, bugs like this would be caught in testing but people don’t necessarily have the hardware to test properly. Or people say, “Oops. I did test this patch but I accidentally sent the wrong version.”

The thing is that when the code is obviously buggy, that takes very little time to address. It’s a one liner patch. The real frustration is when the code is ambiguous. You can stare at the code for a long time and then eventually you decide it works correctly. This is time consuming and there is no reward at the end.

Instead take a look at this code:

ret = short_cut();
if (!ret)
        return 0;
frob();
frob();

When it’s written like this, it’s obvious that the author returned zero intentionally and that it’s a success path.