Am 15.05.2012 16:18, schrieb Boaz Harrosh:
> On 05/15/2012 04:57 PM, Dan Carpenter wrote:
>
>> On Tue, May 15, 2012 at 04:48:23PM +0300, Boaz Harrosh wrote:
>>> On 05/14/2012 10:45 PM, Dan Carpenter wrote:
>>>
>>>> Obviously we should check for NULL here instead of IS_ERR().
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>>>>
>>>> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
>>>> index ba3019f..233beea 100644
>>>> --- a/fs/nfs/idmap.c
>>>> +++ b/fs/nfs/idmap.c
>>>> @@ -644,14 +644,14 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
>>>>
>>>> /* msg and im are freed in idmap_pipe_destroy_msg */
>>>> msg = kmalloc(sizeof(*msg), GFP_KERNEL);
>>>> - if (IS_ERR(msg)) {
>>>> - ret = PTR_ERR(msg);
>>>> + if (!msg) {
>>>
>>>
>>> While at it please put an unlikely()
>>>
>>
>> Normally we wouldn't put an unlikely() here. It makes the code
>> less readable and it's not going to affect benchmarks. But I can
>> add one if people prefer.
>>
>
>
> Personally It makes it more readable for me. It's like a statement:
> "error, always slow-path case here". I have brain parsers set
> for these.
>
> Specifically here the if () is very small but if it is more code
> it is exactly where it counts. But as a general rule I like the
> error/slow-path case to be in an unlikely(). Later someone
> might add more code which will matter.
>
>> regards,
>> dan carpenter
>>
>
>
Not long a go we had a hint from one of the developers that this likely()-stuff
should be used in the scheduler and has no use outside.
re,
wh
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
