linux-kernel - Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Oct 2012 19:08:02 +0200
From:	Peter Senna Tschudin <peter.senna@...il.com>
To:	Julia Lawall <julia.lawall@...6.fr>
Cc:	Joe Perches <joe@...ches.com>, David Miller <davem@...emloft.net>,
	shemminger@...tta.com, mlindner@...vell.com,
	kernel-janitors@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error
 return code

Stephen and David,

I've sent V2 of the patches and they were all accepted. Thank you.

I've made a template for the commit message, and then copy and paste
function names from the code. Something like:

-- // --
The function sky2_probe() return 0 for success and negative value
for most of its internal tests failures. There are two exceptions
that are error cases going to err_out*:. For this two cases, the
function abort its success execution path, but returns non negative
value, making it dificult for a caller function to notice the error.

This patch fixes the error cases that do not return negative values.

This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
...
--//--

How useful it was to have the function names when you were analyzing
the patches? It took me a lot of time to modify the template by copy
and paste, check if it is correct, then commit. I have some other
similar patches to submit and I wonder if having the function names in
the commit message helped you.

Thank you,

Peter


On Fri, Oct 5, 2012 at 10:08 AM, Julia Lawall <julia.lawall@...6.fr> wrote:
> On Fri, 5 Oct 2012, Joe Perches wrote:
>
>> On Fri, 2012-10-05 at 07:22 +0200, Julia Lawall wrote:
>>>
>>> A tool was used to find a potential problem, and then Peter
>>> studied the code to see what fix was appropriate.
>>
>>
>> Hi Julia.
>>
>> Was it true that a static analysis tool found the original
>> potential issue?  If so, what tool was it?
>
>
> In the very beginning, I think that I found the problem in a patch when
> looking at patches that contain oopses.
>
> From that I wrote a Coccinelle rule.  As Peter showed, the rule just
> produces a list of line numbers.  The fix cannot easily be automated,
> because there are many cases where 0 is a valid error value.  Some
> functions, for example, have their error value as a nonpositive integer.
>
>
>> But wasn't the scripted fix applied to the rest of the tree
>> robotically?
>
>
> No.  Peter studied each case and considered what should be done, and then
> did that.  I guess a potentially bad fix could have been applied
> automatically and then cleaned up manually, but considering the number of
> cases where the fix would be wrong, that seem like a bad idea.  Also one
> might want to adapt a bit to local conventions about where the
> initialization should be added.
>
> julia



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