[patch] make hdrscheck.sh force __asm__ in exported headers

[patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

This updates scripts/hdrschecks.sh by grepping for asm() constructs and
rejecting them in favor of __asm__() in exported headers.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/scripts/hdrcheck.sh b/scripts/hdrcheck.sh
index 3159858..33d17cc 100755
--- a/scripts/hdrcheck.sh
+++ b/scripts/hdrcheck.sh
@@ -6,5 +6,16 @@ for FILE in `grep '^[ \t]*#[ \t]*include[ \t]*<' $2 | cut -f2 -d\< | cut -f1 -d\
 	exit 1
     fi
 done
+
+# make sure we export __asm__(), not asm()
+lines=$(grep -n -E \
+	-e '\<asm[[:space:]]*(_{1,2}?volatile_{1,2}?[[:space:]]*)?\(' \
+	$2)
+if [ -n "$lines" ] ; then
+	echo "$2 should convert asm() usage to __asm__() in exported headers"
+	echo "$lines"
+	exit 1
+fi
+
 # FIXME: List dependencies into $3
 touch $3

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Andrew Morton]

On Sun, 17 Jun 2007 18:54:24 -0400 Mike Frysinger <vapier@gentoo.org> wrote:

> This updates scripts/hdrschecks.sh by grepping for asm() constructs and
> rejecting them in favor of __asm__() in exported headers.

And does the kernel pass these checks?  Which architectures have been tested?

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On Monday 18 June 2007, Andrew Morton wrote:
> On Sun, 17 Jun 2007 18:54:24 -0400 Mike Frysinger <vapier@gentoo.org> wrote:
> > This updates scripts/hdrschecks.sh by grepping for asm() constructs and
> > rejecting them in favor of __asm__() in exported headers.
>
> And does the kernel pass these checks?

nope ... should i audit the arches before this gets merged ?

> Which architectures have been tested?

i386 passes, blackfin/x86_64 fail a simple asm/unistd.h, and x86_64 fails a 
bunch in asm/msr.h, but last time i tried to patch that, the maintainer said 
they were going to be hiding a bunch of that stuff anyways
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Andrew Morton]

On Mon, 18 Jun 2007 01:17:46 -0400 Mike Frysinger <vapier@gentoo.org> wrote:

> On Monday 18 June 2007, Andrew Morton wrote:
> > On Sun, 17 Jun 2007 18:54:24 -0400 Mike Frysinger <vapier@gentoo.org> wrote:
> > > This updates scripts/hdrschecks.sh by grepping for asm() constructs and
> > > rejecting them in favor of __asm__() in exported headers.
> >
> > And does the kernel pass these checks?
> 
> nope ... should i audit the arches before this gets merged ?

Yes please.  I'd rather not break things in this fashion: it causes a storm
of emails which I need to redirect to the appropriate maintainers who then
take an arbitrarily long time to do anything.  Meanwhile lots of testers
get impacted and this reduces the testing level of all the other thousands
of patches in there.

> > Which architectures have been tested?
> 
> i386 passes, blackfin/x86_64 fail a simple asm/unistd.h, and x86_64 fails a 
> bunch in asm/msr.h, but last time i tried to patch that, the maintainer said 
> they were going to be hiding a bunch of that stuff anyways

Well I would prefer that we merge as many fixes as poss into -mm before
merging this patch - to reduce the impact on testers.

If these fixes are "wrong" but work then that's fine.  I just keep spamming
the maintainer with the wrong-but-works patch until he wakes up and fixes
it for real.


Also, your changelog sucks:

  This updates scripts/hdrschecks.sh by grepping for asm() constructs and
  rejecting them in favor of __asm__() in exported headers.

well OK.  But for what reason?  We can see a lot of pain in this patch but
no gain.  The changelog should tell us what the benefit is, so we can make
informed tradeoff decisions.

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On Monday 18 June 2007, Andrew Morton wrote:
> On Mon, 18 Jun 2007 01:17:46 -0400 Mike Frysinger <vapier@gentoo.org> wrote:
> > On Monday 18 June 2007, Andrew Morton wrote:
> > > On 17 Jun 2007 18:54:24 -0400 Mike Frysinger <vapier@gentoo.org> wrote:
> > > > This updates scripts/hdrschecks.sh by grepping for asm() constructs
> > > > and rejecting them in favor of __asm__() in exported headers.
> > >
> > > And does the kernel pass these checks?
> >
> > nope ... should i audit the arches before this gets merged ?
>
> Yes please.  I'd rather not break things in this fashion: it causes a storm
> of emails which I need to redirect to the appropriate maintainers who then
> take an arbitrarily long time to do anything.  Meanwhile lots of testers
> get impacted and this reduces the testing level of all the other thousands
> of patches in there.

makes sense, i'll post some stuff

> Also, your changelog sucks:
>
>   This updates scripts/hdrschecks.sh by grepping for asm() constructs and
>   rejecting them in favor of __asm__() in exported headers.
>
> well OK.  But for what reason?  We can see a lot of pain in this patch but
> no gain.  The changelog should tell us what the benefit is, so we can make
> informed tradeoff decisions.

sorry, i blame GNU changelog style ;)
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Monday 18 June 2007, Andrew Morton wrote:
> Also, your changelog sucks:
>
>   This updates scripts/hdrschecks.sh by grepping for asm() constructs and
>   rejecting them in favor of __asm__() in exported headers.
>
> well OK.  But for what reason?  We can see a lot of pain in this patch but
> no gain.  The changelog should tell us what the benefit is, so we can make
> informed tradeoff decisions.

err, should have included the rationale in previous e-mail ...

we want to audit asm/volatile because both constructs will fail in userland 
builds when GNU extensions are turned off (like -std=c99) ... __asm__ and 
__volatile__ will work in both places.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

On 6/18/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon, 18 Jun 2007 01:17:46 -0400 Mike Frysinger <vapier@gentoo.org> wrote:
> > On Monday 18 June 2007, Andrew Morton wrote:
> > > And does the kernel pass these checks?
> >
> > nope ... should i audit the arches before this gets merged ?
>
> Yes please.  I'd rather not break things in this fashion: it causes a storm
> of emails which I need to redirect to the appropriate maintainers who then
> take an arbitrarily long time to do anything.  Meanwhile lots of testers
> get impacted and this reduces the testing level of all the other thousands
> of patches in there.

with the few patches i just sent out, the following pass:
alpha arm avr32 blackfin i386 ia64 m68k mips powerpc s390 sh sparc x86_64

cris and parisc fail vanilla git regardless
-mike

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Sam Ravnborg]

On Sun, Jun 17, 2007 at 06:54:24PM -0400, Mike Frysinger wrote:
> This updates scripts/hdrschecks.sh by grepping for asm() constructs and
> rejecting them in favor of __asm__() in exported headers.

Hi Mike.

I have a new version of hdrcheck in works written in perl
and a bit faster too.
I hope to finish it off this weekend and sent it for review.
Could I ask you to pick it up and add the below test to that version
when it hits mainline - assuming that the other patches has hit mainline too.
I do not want this check before we are clean so we basically use it to
check that noone introduce a bug.

And next time please put some effort in the _why_ part more than the
_how_ part of the changelog.

	Sam

Re: [patch] make hdrscheck.sh force __asm__ in exported headers

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]

On Monday 18 June 2007, Sam Ravnborg wrote:
> On Sun, Jun 17, 2007 at 06:54:24PM -0400, Mike Frysinger wrote:
> > This updates scripts/hdrschecks.sh by grepping for asm() constructs and
> > rejecting them in favor of __asm__() in exported headers.
>
> I have a new version of hdrcheck in works written in perl
> and a bit faster too.
> I hope to finish it off this weekend and sent it for review.
> Could I ask you to pick it up and add the below test to that version
> when it hits mainline - assuming that the other patches has hit mainline
> too. I do not want this check before we are clean so we basically use it to
> check that noone introduce a bug.

that's fine ... one of the other things i wanted to add were checks to make 
sure the exported headers did not contain [us](8|16|32|64) references, but 
there's headers which refer to these in comment blocks or use them as members 
of structs (like the netfilter code) so i couldnt add that check without 
passing things through the preprocessor ... perhaps your perl code would 
better accommodate this, perhaps not ;)
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]