Catalin Marinas wrote:
> From: Catalin Marinas <[email protected]>
[]
> - 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
