mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sam Ravnborg <sam@ravnborg.org>,
	Jaswinder Singh Rajput <jaswinderrajput@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [mingo@elte.hu: [git pull] headers_check fixes]
Date: Tue, 27 Jan 2009 14:57:23 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.0901271441280.3123@localhost.localdomain> (raw)
In-Reply-To: <20090127222825.GA27097@elte.hu>



On Tue, 27 Jan 2009, Ingo Molnar wrote:
> 
> Should i perhaps not bother with the stuff below? Cannot turn off 
> CONFIG_HEADERS_CHECK in my builds because it can cause build failures.

I really hate the patch. I think it's fundamentally flawed. I hate scripts 
that test for things that are readable, and encourage people to then write 
crap instead.

The thing is, the headers_check stuff is just wrong if it causes these 
things, and I'd rather just turn it off.

If those 

	#ifdef CONFIG_XYZ

things result in problems, then we should just make the rule be that we 
turn that kind of string into

	#if 0

automatically when exporting the kernel headers. IOW, just about 
_anything_ that headers_check complains about automatically is something 
that should just be _fixed_ automatically at header install time rather 
than make the code harder to read.

So I think it makes our headers worse. Code like

	> +#ifdef __KERNEL__
	> +# ifdef CONFIG_X86_BSWAP
	> +# define __X86_BSWAP  
	> +# endif /* CONFIG_X86_BSWAP */
	> +#endif /* __KERNEL__ */

just doesn't make sense. It doesn't make sense _inside_ the kernel, and it 
doesn't make sense _outside_ it either.

As far as I can tell, the header install script could literally just do 
something like run 'sed' over the headers as it installs them, and do 
something like

	sed 's/\<CONFIG_[A-Z0-9_]*\>/__kernel_only__/g'

which I realize is not really the complete/correct solution (ie you could 
write a nicer thing that does a better job), but my point here is that 
rather than have scripts that _whine_ about these kinds of trivial things 
and cause people to write less readable header files, we should just make 
sure that if we can recognize them so easily, we can just fix them 
instead.

End result: headers that don't suck.

If the damn headers-check isn't working for people, then let's turn the 
thing off, not make our code look worse.

There are parts of the patches that look fine, like moving __KERNEL__ 
checks around a bit, and changing <asm/types.h> to <linux/types.h> which 
looks correct _both_ in a kernel and in a user context, but I dislike the 
stupid parts.

			Linus

       reply	other threads:[~2009-01-27 22:57 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090127222825.GA27097@elte.hu>
2009-01-27 22:57 ` Linus Torvalds [this message]
2009-01-27 23:22   ` H. Peter Anvin
2009-01-27 23:29     ` Linus Torvalds
2009-01-28  0:12       ` H. Peter Anvin
2009-01-28  0:19         ` Linus Torvalds
2009-01-28  1:02           ` H. Peter Anvin
2009-01-27 23:31   ` Ingo Molnar
2009-01-27 23:43     ` Linus Torvalds
2009-01-27 23:51     ` Vegard Nossum
2009-01-30 14:01     ` Jaswinder Singh Rajput
2009-01-30 18:20       ` Ingo Molnar
2009-01-28  0:03   ` Harvey Harrison
2009-01-28  1:36   ` Jaswinder Singh Rajput
2009-01-28 12:37     ` Arnd Bergmann
2009-01-28 17:48       ` H. Peter Anvin
2009-01-28 19:22         ` Harvey Harrison
2009-01-28 19:44           ` Linus Torvalds
2009-01-28 20:03             ` Harvey Harrison
2009-01-28 21:25               ` H. Peter Anvin
2009-01-28 21:58                 ` [PATCH] x86: do not expose CONFIG_BSWAP to userspace Harvey Harrison
2009-01-28 22:13                   ` Linus Torvalds
2009-01-28 22:40                     ` Harvey Harrison
2009-01-30 20:37                       ` Pavel Machek
2009-01-28 22:15                   ` H. Peter Anvin
2009-01-28 22:38                     ` Harvey Harrison
2009-01-28 23:04                       ` Ben Pfaff
2009-01-30 18:20                         ` H. Peter Anvin
2009-01-28 23:27                       ` H. Peter Anvin
2009-01-28 23:36                         ` Harvey Harrison
2009-01-28 23:47                           ` H. Peter Anvin
2009-02-03 18:19                             ` Arnd Bergmann
2009-01-31 18:43                       ` Maciej W. Rozycki
2009-01-31 20:24                         ` H. Peter Anvin
2009-01-28 23:24                     ` Arnd Bergmann
2009-01-28 23:30                       ` H. Peter Anvin
2009-01-28 20:49             ` [mingo@elte.hu: [git pull] headers_check fixes] Sam Ravnborg
2009-01-28 21:23           ` H. Peter Anvin
2009-01-28 21:06   ` Sam Ravnborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.00.0901271441280.3123@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=jaswinderrajput@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®