lkml.org 
[lkml]   [2006]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subjectfunctions returning 0 on success [was: [PATCH] Fix a memory leak in the i386 setup code]
Catalin Marinas wrote:
> From: Catalin Marinas <catalin.marinas@gmail.com>
[]
> - request_resource(&iomem_resource, res);
> + if (request_resource(&iomem_resource, res)) {
> + kfree(res);
...

Just a small nitpick, or, rather, a question.
Quite alot of functions return 0 on success, or !=0 on failure.
There are other functions, which, say, return NULL on failure.
Or when 0 is valid result, something <0 is returned.. etc.

Without looking at the function description or code, it's
impossible to say wich of the above it is. But.

When reading that kind of code as the quoted patch adds,
I for one always think it's somehow incorrect or backwards.
When used like that, request_resource() seems like a boolean,
and the whole thing becomes:

if (do_something_successeful())
fail();

Ofcourse, later you understand that do_something() returns 0
on failure, and the code is correct. But the first impression
(again, for me anyway) is that it's wrong.

In such cases when a routine returns 0 on error, I usually write
it this way:

if (request_resource() != 0)
fail()

This way it becomes obvious what does it do, and compiler
generates EXACTLY the same instructions.

Yes it's redundrand, that "!= 0" tail. But it makes the
whole stuff readable without a need to re-think what does it
return, and, which is especially important, logically correct
when reading.

Alternatively, it can be named something like

request_resource_failed()

instead of just request_resource(). Compare:

if (request_resource(..))
fail();

(current). And

if (request_resource_failed())
fail();

The latter seems more logical... ;)

But that "!= 0" tail achieves almost the same effect.
Yet again, from my point of view anyway... ;)

Thanks.

/mjt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-07-11 00:49    [W:0.181 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site