[PATCH 0/9] Kconfig: cleanup s390 v2.

[PATCH 0/9] Kconfig: cleanup s390 v2.

[Martin Schwidefsky]

Greetings,
I've added the results of the review to the Kconfig cleanup patches
for s390. Patch #2 has been split, one half has all the HAS_IOMEM
depends lines the other the remaining !S390 depends lines.

Andrew: I plan to add patches 1-5 to the for-andrew branch of the
git390 repository if that is fine with you. The only thing that will
be missing in the tree is the patch that disables wireless for s390.
The code does compile but without hardware it is mute to have the
config options. I'll wait until the git-wireless.patch is upstream.
Patches 7-9 depend on patches found in -mm.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Arnd Bergmann]

On Monday 23 April 2007, Martin Schwidefsky wrote:
> I've added the results of the review to the Kconfig cleanup patches
> for s390. Patch #2 has been split, one half has all the HAS_IOMEM
> depends lines the other the remaining !S390 depends lines.
> 

They all look good to me now

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Mon, 23 Apr 2007 16:11:23 +0200
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> Greetings,
> I've added the results of the review to the Kconfig cleanup patches
> for s390. Patch #2 has been split, one half has all the HAS_IOMEM
> depends lines the other the remaining !S390 depends lines.
> 
> Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> git390 repository if that is fine with you. The only thing that will
> be missing in the tree is the patch that disables wireless for s390.
> The code does compile but without hardware it is mute to have the
> config options. I'll wait until the git-wireless.patch is upstream.
> Patches 7-9 depend on patches found in -mm.
> 

umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.

Over-full, really: I've been working basically continuously since Friday
getting the current dungpile to compile and boot, and it's still miles away
from that.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Martin Schwidefsky]

On Mon, 2007-04-23 at 10:45 -0700, Andrew Morton wrote:
> > Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> > git390 repository if that is fine with you. The only thing that will
> > be missing in the tree is the patch that disables wireless for s390.
> > The code does compile but without hardware it is mute to have the
> > config options. I'll wait until the git-wireless.patch is upstream.
> > Patches 7-9 depend on patches found in -mm.
> > 
> 
> umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
> 
> Over-full, really: I've been working basically continuously since Friday
> getting the current dungpile to compile and boot, and it's still miles away
> from that.

I understand. I'll wait until -mm is a little bit smaller again. It is
just that someday I want to finish with the Kconfig cleanup, it has been
sitting on my harddriver for ages now.

-- 
blue skies,              IBM Deutschland Entwicklung GmbH
   Martin                Vorsitzender des Aufsichtsrats: Johann Weihen
                         Geschäftsführung: Herbert Kircher
Martin Schwidefsky       Sitz der Gesellschaft: Böblingen
Linux on zSeries         Registergericht: Amtsgericht Stuttgart,
   Development           HRB 243294

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Randy Dunlap]

On Mon, 23 Apr 2007 10:45:34 -0700 Andrew Morton wrote:

> On Mon, 23 Apr 2007 16:11:23 +0200
> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > Greetings,
> > I've added the results of the review to the Kconfig cleanup patches
> > for s390. Patch #2 has been split, one half has all the HAS_IOMEM
> > depends lines the other the remaining !S390 depends lines.
> > 
> > Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> > git390 repository if that is fine with you. The only thing that will
> > be missing in the tree is the patch that disables wireless for s390.
> > The code does compile but without hardware it is mute to have the
> > config options. I'll wait until the git-wireless.patch is upstream.
> > Patches 7-9 depend on patches found in -mm.
> > 
> 
> umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
> 
> Over-full, really: I've been working basically continuously since Friday
> getting the current dungpile to compile and boot, and it's still miles away
> from that.

and I continue to be concerned about the amount of patch reviews
compared to new patch material overall (not just s390).

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Wed, 25 Apr 2007 11:21:33 -0700
Randy Dunlap <randy.dunlap@oracle.com> wrote:

> On Mon, 23 Apr 2007 10:45:34 -0700 Andrew Morton wrote:
> 
> > On Mon, 23 Apr 2007 16:11:23 +0200
> > Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> > 
> > > Greetings,
> > > I've added the results of the review to the Kconfig cleanup patches
> > > for s390. Patch #2 has been split, one half has all the HAS_IOMEM
> > > depends lines the other the remaining !S390 depends lines.
> > > 
> > > Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> > > git390 repository if that is fine with you. The only thing that will
> > > be missing in the tree is the patch that disables wireless for s390.
> > > The code does compile but without hardware it is mute to have the
> > > config options. I'll wait until the git-wireless.patch is upstream.
> > > Patches 7-9 depend on patches found in -mm.
> > > 
> > 
> > umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
> > 
> > Over-full, really: I've been working basically continuously since Friday
> > getting the current dungpile to compile and boot, and it's still miles away
> > from that.
> 
> and I continue to be concerned about the amount of patch reviews
> compared to new patch material overall (not just s390).
> 

yes.  I'm increasingly reluctant to merge things which have had no visible
review from any third party.  Nowadays I'll shove such patches into a
pending folder and will wait a day or three to see if anyone has any
feedback.  If they don't I have to either ignore the patches or review them
myself.

I expect (and hope) that more formal processes will come about here.  Perhaps
up to it-won't-be-merged-without-a-Reviewed-by:.


But that only applies to things which I merge.  There's heaps of stuff
coming in via the git trees which is obviously inadequately reviewed - look
at all the instances of open-coded kernel_thread() which were merged after
the kthread() API was introduced, for example.


And other basic stuff like "use mutexes, not semaphores":

box:/usr/src/25> grep '^+.*[    ]down[  ]*[(]' patches/git-*.patch | wc -l
32



Ever wonder where all those whitespace bugs are coming from?

box:/usr/src/25> grep '^+.*[    ]if[(]' patches/git-*.patch | wc -l
265
box:/usr/src/25> grep '^+.*[    ]while[(]' patches/git-*.patch | wc -l  
35


Code which use spaces where it should be using tabs?

box:/usr/src/25> grep '^+        ' patches/git-*.patch | wc -l
1346


Heaven knows how many more serious problems are being snuck into the tree
via this route.

What do we do?

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Wed, 25 Apr 2007 14:30:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> But that only applies to things which I merge.  There's heaps of stuff
> coming in via the git trees which is obviously inadequately reviewed - look
> at all the instances of open-coded kernel_thread() which were merged after
> the kthread() API was introduced, for example.
> 
> 
> And other basic stuff like "use mutexes, not semaphores":
> 
> box:/usr/src/25> grep '^+.*[    ]down[  ]*[(]' patches/git-*.patch | wc -l
> 32
> 
> 
> 
> Ever wonder where all those whitespace bugs are coming from?
> 
> box:/usr/src/25> grep '^+.*[    ]if[(]' patches/git-*.patch | wc -l
> 265
> box:/usr/src/25> grep '^+.*[    ]while[(]' patches/git-*.patch | wc -l  
> 35
> 
> 
> Code which use spaces where it should be using tabs?
> 
> box:/usr/src/25> grep '^+        ' patches/git-*.patch | wc -l
> 1346
> 

It would be neat if someone could create and maintain a new
scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
about newly-added code (and only newly-added code) which has busted
whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.

It would need to be fairly simple and easily-extensible, as I can
imagine quite a few things getting added to it.

(Imagines a procmail rule which just bounces the email if
spot-common-mistakes failed)


> 
> Heaven knows how many more serious problems are being snuck into the tree
> via this route.

But it won't solve this problem.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Arnd Bergmann]

On Thursday 26 April 2007, Andrew Morton wrote:
> It would be neat if someone could create and maintain a new
> scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> about newly-added code (and only newly-added code) which has busted
> whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.

http://patchstylecheck.googlecode.com/svn/trunk/patchstylecheckemail.pl
Might serve as a starting point for this. It doesn't have any semantic
checks right now, but I guess they can be added.

	Arnd <><

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Dave Jones]

On Wed, Apr 25, 2007 at 05:24:47PM -0700, Andrew Morton wrote:

 > It would be neat if someone could create and maintain a new
 > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
 > about newly-added code (and only newly-added code) which has busted
 > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.

years and years ago, when the dinosaurs roamed the land, I hacked up..
http://janitor.kernelnewbies.org/scripts/  and then left it by the wayside.
Some of the checks it did are actually bogus, but I'm happy to pick that
up again if there's interest in it being a useful tool.

In fact, I should probably munge it together with a similar thing
I wrote at http://www.codemonkey.org.uk/projects/findbugs/
(Warning: scary regexps)

 > It would need to be fairly simple and easily-extensible, as I can
 > imagine quite a few things getting added to it.
 > 
 > (Imagines a procmail rule which just bounces the email if
 > spot-common-mistakes failed)

or a git checkin rule that refuses to commit if it fails ;-)

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Thu, 26 Apr 2007 02:32:06 +0200 Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday 26 April 2007, Andrew Morton wrote:
> > It would be neat if someone could create and maintain a new
> > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> > about newly-added code (and only newly-added code) which has busted
> > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> 
> http://patchstylecheck.googlecode.com/svn/trunk/patchstylecheckemail.pl
> Might serve as a starting point for this. It doesn't have any semantic
> checks right now, but I guess they can be added.
> 

	print "Your patch is now worthy to be reviewed by a real person\n";

heh.  Yes, that looks like an ideal starting point.

Methinks it should do `exit 1' if anything was detected.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Anton Vorontsov]

On Thu, Apr 26, 2007 at 02:32:06AM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Andrew Morton wrote:
> > It would be neat if someone could create and maintain a new
> > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> > about newly-added code (and only newly-added code) which has busted
> > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> 
> http://patchstylecheck.googlecode.com/svn/trunk/patchstylecheckemail.pl
> Might serve as a starting point for this. It doesn't have any semantic
> checks right now, but I guess they can be added.

Had run this utility against my battery patches, and caught
bunch of false positives (I believe).


+#define BATTERY_PROP(bat, prop) ({                                 \
+	void *value = bat->get_property(bat, BATTERY_PROP_##prop); \
+	value ? *(int*)value : 0;                                  \
+})

Got: "Macros with multiple statements should be enclosed in a do - while
loop"

I believed ({}) is equivalent for "do - while", it's widely used in
kernel.


+	switch (bp) {
+	default: break;
+	};

Got "Gotos should not be indented", at "default: break;"


+static int bind_pst_to_psy(struct power_supplicant *pst,
+                           struct power_supply *psy)
+{

Got "use tabs not spaces". Here spaces intentionally used for
formatting purpose, not for the indenting.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Randy Dunlap]

Dave Jones wrote:
> On Wed, Apr 25, 2007 at 05:24:47PM -0700, Andrew Morton wrote:
> 
>  > It would be neat if someone could create and maintain a new
>  > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
>  > about newly-added code (and only newly-added code) which has busted
>  > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> 
> years and years ago, when the dinosaurs roamed the land, I hacked up..
> http://janitor.kernelnewbies.org/scripts/  and then left it by the wayside.
> Some of the checks it did are actually bogus, but I'm happy to pick that
> up again if there's interest in it being a useful tool.
> 
> In fact, I should probably munge it together with a similar thing
> I wrote at http://www.codemonkey.org.uk/projects/findbugs/
> (Warning: scary regexps)
> 
>  > It would need to be fairly simple and easily-extensible, as I can
>  > imagine quite a few things getting added to it.
>  > 
>  > (Imagines a procmail rule which just bounces the email if
>  > spot-common-mistakes failed)
> 
> or a git checkin rule that refuses to commit if it fails ;-)

Yep, I was going to mention your scripts but you beat me to it.

I'll be glad to help maintain such animals if wanted.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Wed, 25 Apr 2007 19:38:23 -0700 Randy Dunlap <randy.dunlap@oracle.com> wrote:

> Dave Jones wrote:
> > On Wed, Apr 25, 2007 at 05:24:47PM -0700, Andrew Morton wrote:
> > 
> >  > It would be neat if someone could create and maintain a new
> >  > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> >  > about newly-added code (and only newly-added code) which has busted
> >  > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> > 
> > years and years ago, when the dinosaurs roamed the land, I hacked up..
> > http://janitor.kernelnewbies.org/scripts/  and then left it by the wayside.
> > Some of the checks it did are actually bogus, but I'm happy to pick that
> > up again if there's interest in it being a useful tool.
> > 
> > In fact, I should probably munge it together with a similar thing
> > I wrote at http://www.codemonkey.org.uk/projects/findbugs/
> > (Warning: scary regexps)
> > 
> >  > It would need to be fairly simple and easily-extensible, as I can
> >  > imagine quite a few things getting added to it.
> >  > 
> >  > (Imagines a procmail rule which just bounces the email if
> >  > spot-common-mistakes failed)
> > 
> > or a git checkin rule that refuses to commit if it fails ;-)
> 
> Yep, I was going to mention your scripts but you beat me to it.
> 
> I'll be glad to help maintain such animals if wanted.
> 

wanted ;)

At least, it would be interesting to investigate the usefulness.  I suspect
it will prove to be very useful for the little things.

Heck, someone could subscribe a robot to all the mailing lists which sends
nastygrams straight back at people who submit broken patches.  We already
need that for tab-replaced and word-wrapped patches.  (ok, we have it -
it's called akpm, but being robotic wearies one)

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Dave Jones]

On Wed, Apr 25, 2007 at 08:02:07PM -0700, Andrew Morton wrote:
 > On Wed, 25 Apr 2007 19:38:23 -0700 Randy Dunlap <randy.dunlap@oracle.com> wrote:
 > > > In fact, I should probably munge it together with a similar thing
 > > > I wrote at http://www.codemonkey.org.uk/projects/findbugs/
 > > > (Warning: scary regexps)
 > > I'll be glad to help maintain such animals if wanted.
 > 
 > wanted ;)
 > 
 > At least, it would be interesting to investigate the usefulness.  I suspect
 > it will prove to be very useful for the little things.

Yeah, the original script tried to do things like spinlock balancing checks,
(badly). This was long before had sparse, and it was partly a "lets learn some perl"
experience for myself. I'll toss that idea out now that we have better tools
for that, and keep it to simple checks.

 > Heck, someone could subscribe a robot to all the mailing lists which sends
 > nastygrams straight back at people who submit broken patches.  We already
 > need that for tab-replaced and word-wrapped patches.  (ok, we have it -
 > it's called akpm, but being robotic wearies one)

Ok, I've got a few different flavours of that script. I'll roll them
all into one tomorrow and throw out some of the noisy silly ones
(I don't think warning about strcpy->strncpy is really worthwhile for eg).

Additional regexps gratefully recieved.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Thu, 26 Apr 2007 02:32:06 +0200 Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday 26 April 2007, Andrew Morton wrote:
> > It would be neat if someone could create and maintain a new
> > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> > about newly-added code (and only newly-added code) which has busted
> > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> 
> http://patchstylecheck.googlecode.com/svn/trunk/patchstylecheckemail.pl
> Might serve as a starting point for this. It doesn't have any semantic
> checks right now, but I guess they can be added.

oh man, every patch I review, every bug I fix, I dream of this.



Wishlist:

- wire it up to a robot which monitors all Linux mailing lists and sends
  machine-review comments back to originators.  This will be a huge win by
  eliminating so much stupid crap.  

- auto-detect wordwrapped and tab-replaced emails (oh glory)

- auto-detect code wider than 80-cols (swoon)

- auto-detect missing Signed-off-by:

- auto-check patch format and protocol, as per
  http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt and
  http://linux.yyz.us/patch-format.html

- teach it about semantics:

  - kthread instead of kernel_thread

  - mutexes instead of semaphores

  - the whole plethora of whitespace uckfuppednesses

  - needlessly-initialised-to-zero-static-variables

  - extern-decls-in-C

  - EXPORT_SYMBOL(foo) is placed immediately after foo()'s closing
    brace

    Hard to do?  Could just whine about all EXPORT_SYMBOL's which
    aren't immediately preceded by ^}$ or by ;$

  - new typedefs

  - use of uint32_t and friends

  - use of BUG_ON and BUG, frankly.  The thing's a damn pest.  Suggest
    WARN_ON+recover-from-it.

  - use of `if ((var = expr()))' and similar

  - large inlined functions?

  - braces around single statements (advanced topic ;))

  - StudlyCaps?

  - anything called "tmp" or "temp"

  - old-style struct initialisers

  - non-ascii characters(?)

  - lots more to come, I'm sure.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andy Whitcroft]

Andrew Morton wrote:
> On Wed, 25 Apr 2007 11:21:33 -0700
> Randy Dunlap <randy.dunlap@oracle.com> wrote:
> 
>> On Mon, 23 Apr 2007 10:45:34 -0700 Andrew Morton wrote:
>>
>>> On Mon, 23 Apr 2007 16:11:23 +0200
>>> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>>>
>>>> Greetings,
>>>> I've added the results of the review to the Kconfig cleanup patches
>>>> for s390. Patch #2 has been split, one half has all the HAS_IOMEM
>>>> depends lines the other the remaining !S390 depends lines.
>>>>
>>>> Andrew: I plan to add patches 1-5 to the for-andrew branch of the
>>>> git390 repository if that is fine with you. The only thing that will
>>>> be missing in the tree is the patch that disables wireless for s390.
>>>> The code does compile but without hardware it is mute to have the
>>>> config options. I'll wait until the git-wireless.patch is upstream.
>>>> Patches 7-9 depend on patches found in -mm.
>>>>
>>> umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
>>>
>>> Over-full, really: I've been working basically continuously since Friday
>>> getting the current dungpile to compile and boot, and it's still miles away
>>> from that.
>> and I continue to be concerned about the amount of patch reviews
>> compared to new patch material overall (not just s390).
>>
> 
> yes.  I'm increasingly reluctant to merge things which have had no visible
> review from any third party.  Nowadays I'll shove such patches into a
> pending folder and will wait a day or three to see if anyone has any
> feedback.  If they don't I have to either ignore the patches or review them
> myself.
> 
> I expect (and hope) that more formal processes will come about here.  Perhaps
> up to it-won't-be-merged-without-a-Reviewed-by:.

Is this not the meaning of the Acked-by: ?

> Heaven knows how many more serious problems are being snuck into the tree
> via this route.
> 
> What do we do?

Perhaps its time for Linus to say he won't accept any patches which are
not Acked and place the onus on getting those on the tree maintainers.
In theory at least tree maintainers are supposed to be responsible for
the stuff coming through their tree.  They could be made responsible to
ensuring only Ack'd stuff is committed.  Automated checks could be made
for that at least.

As for the white space errors.  I think that we should perhaps run a
spectrum of commits from each tree through the checked proposed later in
the thread.  Where any significant non-compliance is detected that
should be sent to the tree maintainer and their 'upstream' and
corrections expected.

Public shaming.  A savaging from Linus' or any other respected community
member is something to be avoided at all costs.

-apw

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Randy Dunlap]

On Thu, 26 Apr 2007 01:30:59 -0700 Andrew Morton wrote:

> On Thu, 26 Apr 2007 02:32:06 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Thursday 26 April 2007, Andrew Morton wrote:
> > > It would be neat if someone could create and maintain a new
> > > scripts/spot-common-mistakes.  Feed it a unified diff and it would complain
> > > about newly-added code (and only newly-added code) which has busted
> > > whitespace, adds new semaphores, adds new kernel_thread calls, etc, etc.
> > 
> > http://patchstylecheck.googlecode.com/svn/trunk/patchstylecheckemail.pl
> > Might serve as a starting point for this. It doesn't have any semantic
> > checks right now, but I guess they can be added.
> 
> oh man, every patch I review, every bug I fix, I dream of this.

singing?


> Wishlist:
> 
> - wire it up to a robot which monitors all Linux mailing lists and sends
>   machine-review comments back to originators.  This will be a huge win by
>   eliminating so much stupid crap.  
> 
> - auto-detect wordwrapped and tab-replaced emails (oh glory)
> 
> - auto-detect code wider than 80-cols (swoon)
> 
> - auto-detect missing Signed-off-by:
> 
> - auto-check patch format and protocol, as per
>   http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt and
>   http://linux.yyz.us/patch-format.html
> 
> - teach it about semantics:
> 
>   - kthread instead of kernel_thread
> 
>   - mutexes instead of semaphores
> 
>   - the whole plethora of whitespace uckfuppednesses
> 
>   - needlessly-initialised-to-zero-static-variables
> 
>   - extern-decls-in-C
> 
>   - EXPORT_SYMBOL(foo) is placed immediately after foo()'s closing
>     brace
> 
>     Hard to do?  Could just whine about all EXPORT_SYMBOL's which
>     aren't immediately preceded by ^}$ or by ;$
> 
>   - new typedefs
> 
>   - use of uint32_t and friends
> 
>   - use of BUG_ON and BUG, frankly.  The thing's a damn pest.  Suggest
>     WARN_ON+recover-from-it.
> 
>   - use of `if ((var = expr()))' and similar
> 
>   - large inlined functions?
> 
>   - braces around single statements (advanced topic ;))
> 
>   - StudlyCaps?
> 
>   - anything called "tmp" or "temp"
> 
>   - old-style struct initialisers
> 
>   - non-ascii characters(?)
> 
>   - lots more to come, I'm sure.

I made a short list last night, but yours is better/longer.
Here are a few more items:

- check for new docs and/or kernel-doc
- storage class should be first
- don't use deprecated APIs
- no floating point
- no MIN/MAX
- printk wants KERN_* levels
- limit on new /proc files


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Martin Schwidefsky]

On Mon, 2007-04-23 at 10:45 -0700, Andrew Morton wrote: 
> > Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> > git390 repository if that is fine with you. The only thing that will
> > be missing in the tree is the patch that disables wireless for s390.
> > The code does compile but without hardware it is mute to have the
> > config options. I'll wait until the git-wireless.patch is upstream.
> > Patches 7-9 depend on patches found in -mm.
> > 
> 
> umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
> 
> Over-full, really: I've been working basically continuously since Friday
> getting the current dungpile to compile and boot, and it's still miles away
> from that.

Is -mm less full now? I've seen you dropped your third patch bomb, so
maybe there is room now ..
I regenerated the kconfig patches. They fit on top of todays git but
they do have some rejects on 2.6.21-mm1. The rejects are trivial to fix,
but there are about 10 of them.

So when will be a good time to add the Kconfig patches to git390?
And do you prefer to let them rot^H^H^Hmature in -mm for a while before
they can go upstream?

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Andrew Morton]

On Wed, 09 May 2007 13:21:50 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> On Mon, 2007-04-23 at 10:45 -0700, Andrew Morton wrote: 
> > > Andrew: I plan to add patches 1-5 to the for-andrew branch of the
> > > git390 repository if that is fine with you. The only thing that will
> > > be missing in the tree is the patch that disables wireless for s390.
> > > The code does compile but without hardware it is mute to have the
> > > config options. I'll wait until the git-wireless.patch is upstream.
> > > Patches 7-9 depend on patches found in -mm.
> > > 
> > 
> > umm, OK.  If it's Ok I think I'll duck it for now: -mm is full.
> > 
> > Over-full, really: I've been working basically continuously since Friday
> > getting the current dungpile to compile and boot, and it's still miles away
> > from that.
> 
> Is -mm less full now? I've seen you dropped your third patch bomb, so
> maybe there is room now ..

Near 500 patches, which is the lowest it's been in years.  I do appear to
have a few hundred patch-bearing emails saved away though.

> I regenerated the kconfig patches. They fit on top of todays git but
> they do have some rejects on 2.6.21-mm1. The rejects are trivial to fix,
> but there are about 10 of them.
> 
> So when will be a good time to add the Kconfig patches to git390?

Go wild.

> And do you prefer to let them rot^H^H^Hmature in -mm for a while before
> they can go upstream?

Your call.  I'd be inclined to push them early personally - Kconfig
problems tend to be easy to spot and easy to fix and there's ample time to
fine tune it.

Re: [PATCH 0/9] Kconfig: cleanup s390 v2.

[Martin Schwidefsky]

On Wed, 2007-05-09 at 09:35 -0700, Andrew Morton wrote:
> > So when will be a good time to add the Kconfig patches to git390?
> 
> Go wild.
> 
> > And do you prefer to let them rot^H^H^Hmature in -mm for a while before
> > they can go upstream?
> 
> Your call.  I'd be inclined to push them early personally - Kconfig
> problems tend to be easy to spot and easy to fix and there's ample time to
> fine tune it.

Ok, that is my feeling as well. The patches won't get better by waiting.
I will add them to the for-linus/for-andrew branches and include them in
the next please-pull for Linus. Be prepared for some breakage in -mm..

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.