Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757380AbZANUNs (ORCPT ); Wed, 14 Jan 2009 15:13:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752650AbZANUNf (ORCPT ); Wed, 14 Jan 2009 15:13:35 -0500 Received: from pfepa.post.tele.dk ([195.41.46.235]:46740 "EHLO pfepa.post.tele.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757363AbZANUNd (ORCPT ); Wed, 14 Jan 2009 15:13:33 -0500 Date: Wed, 14 Jan 2009 21:15:13 +0100 From: Sam Ravnborg To: Geert Uytterhoeven Cc: Jaswinder Singh Rajput , Jaswinder Singh Rajput , Ingo Molnar , Alan Cox , Andrew Morton , David Miller , x86 maintainers , LKML , netdev Subject: Re: [PULL -tip] fixed few make headers_check warnings Message-ID: <20090114201513.GC1556@uranus.ravnborg.org> References: <1231836016.3212.41.camel@jaswinder.satnam> <3f9a31f40901140129i1389f47frf32808f3a695a118@mail.gmail.com> <20090114153825.GB32082@uranus.ravnborg.org> <3f9a31f40901140759s1542e6cclae1808b796c2b1a9@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.1i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2573 Lines: 57 On Wed, Jan 14, 2009 at 05:36:14PM +0100, Geert Uytterhoeven wrote: > On Wed, 14 Jan 2009, Jaswinder Singh Rajput wrote: > > On Wed, Jan 14, 2009 at 9:08 PM, Sam Ravnborg wrote: > > > I appreciate your work but I will like to question the approach. > > > > My approach was: > > "PATCH should solve a problem per file", like: > > capability.h: extern's make no sense in userspace > > coda_psdev.h: extern's make no sense in userspace > > in6.h: extern's make no sense in userspace > > nubus.h: extern's make no sense in userspace > > socket.h: extern's make no sense in userspace > > > > But this warnings was in many files: > > include of is preferred over : 15 files > > found __[us]{8,16,32,64} type without #include : 52 files > > > > So in place of making 15 + 52 = 67 patches, I made 2 patches for each warning. > > > > > We should rather take the warnings as an indication that this > > > file needs to be looked over and fix not only the warnings > > > reported but rater to fix all the questionable issues on a file-by-file basis. > > > > Should I make 67 patches ? > > No. > > What Sam means is that the warnings about externs not making sense in userspace > are indicators that there may be other external declarations (without "extern") > in those files, and that you should fix those at the same time (i.e. either > don't fix any of them, or fix all of them (in the same file)). If you don't > fix them at the same time, people tend to forget about them. > > So the warnings are just considered canaries in our coal mine. Killing only the > canaries doesn't help. Correct. We should use the warnings to tell us that there may be something fishy here and fix it all. I've used checkpatch in the same way occasionally. If I had to do some cleanup and checkpatch was a bit noisy I went through the file _manually_. And when finished I ran chackpatch again and fixed up the warnings/errors that made sense for me to fix and that I forgot when I refactored the code. So when I see "capability.h: extern's make no sense in userspace" the perfect approach would be that the whole file was checked. This is a much bigger task that just removing the warning. (All this said I have sometimes doen the "remove warnings only trick too"). Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/