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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <499984C1.6020004@s5r6.in-berlin.de>
Date:	Mon, 16 Feb 2009 16:22:41 +0100
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Sam Ravnborg <sam@...nborg.org>,
	Manish Katiyar <mkatiyar@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] Remove errors caught by checkpatch.pl in	kernel/kallsyms.c

Ingo Molnar wrote:
> We routinely mention Sparse, lockdep, Coverity, Coccinelle, kmemleak, 
> ftrace, kmemcheck and other tools as well when it motives to fix a bug 
> or uncleanliness. [...] It is absolutely fine to
> mention checkpatch when it catches uncleanliness in code that already 
> got merged. I dont understand your point.

I wrote "don't mention checkpatch" but I really meant "think about what
the effect of the patch is and describe this".

It's not really a hard problem to mention checkpatch --- it is a problem
to make it the main point or, like in this case, the only point of the
changelog.  (Furthermore, it is also a problem to do something routinely
*if* doing it does not make sense.  There routinely appear coccinelle
metapatch sources in changelogs.  That does not make sense at all, and
doing it routinely is not a justification in itself.)

So, "don't mention checkpatch" is simply a rule of thumb; read it as "I
mentioned checkpatch in the changelog --- wait, I have possibly written
a changelog that is besides the point; I should think about it once
more".  :-)

Now, when this particular patch is updated to get a good changelog, then
the title could become e.g.:
	kernel/kallsyms: change initcall level; adjust whitespace
and anything more than that is just fluff and wasted electrons. Actually
the changelog should rather contain a note on why device_initcall is
supposed to be the correct initcall level.

Fixes due to reports from sparse, lockdep, coverity, coccinelle, etc.
are the in this respect the same as fixes due to reports from
checkpatch:  Patch titles should for example be
  - "fix potential deadlock..."
  - "fix use-after free..."
  - "use XYZ helper..."
  - "adjust whitespace..."
and *not* something like "fix lockdep backtrace" or whatever.

A difference would be a patch title like "add sparse annotations"
because this is indeed about what the patch does, not by which means it
was created.

Why do I make a fuzz?  Well, because many of our changelogs really suck
and we need to become better in general.
-- 
Stefan Richter
-=====-=-=== -=-= -==-=
http://arcgraph.de/sr/
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ