Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix errorreturn code
From: David Miller
Date: Thu Oct 04 2012 - 14:54:14 EST
From: Peter Senna Tschudin <peter.senna@xxxxxxxxx>
Date: Thu, 4 Oct 2012 20:49:57 +0200
> On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@xxxxxxxxxxxxx> wrote:
>> From: Peter Senna Tschudin <peter.senna@xxxxxxxxx>
>> Date: Thu, 4 Oct 2012 19:32:12 +0200
>>
>>> I can't understand the advantages of describing each patch as you are
>>> asking. "For me" the generic commit message together with the patch
>>> makes sense. Can you please help me on that?
>>
>> Stop being so dense.
>>
>> We want to know the implications of the bug being fixed.
>>
>> Does it potentially cause an OOPS? Bad reference counting and thus
>> potential leaks or early frees?
>>
>> You have to analyze the implications and ramifications of the bug
>> being fixed. We need that information.
>>
>> Your commit messages are in fact robotic, they don't describe the
>> salient details of what kinds of problems the bug being fixed might
>> cause.
>>
>> It's just "bad error code, this is the script that fixed it, kthx,
>> bye" which is pretty much useless for anaylsis.
>
> Thank you David.
>
> What about this as commit message?
> --- // --
> This patch fixes bug(s) related to return value of function(s). In
> some error cases, the function is returning non-negative SUCCESS
> value, when the correct would be negative ERROR value.
>
> When on error, returning non negatives values, or SUCCESS, breaks error
> propagation, producing unpredictable behavior that are very difficult
> to debug.
> --- // --
What does it potentially cause the caller to do? Will it potentially
treat an error as a success and as a result register an object
illegally?
Real analysis please. The text you provided above is basically still
robotic and could be used to describe any error code return fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/