[patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Mike Frysinger]

This changes asm() to __asm__() and volatile to __volatile__ so that these
headers can be used with gcc's -std=c99.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/include/asm-arm/byteorder.h b/include/asm-arm/byteorder.h
index e6f7fcd..39105dc 100644
--- a/include/asm-arm/byteorder.h
+++ b/include/asm-arm/byteorder.h
@@ -29,7 +29,7 @@ static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
 		 * right thing and not screw it up to different degrees
 		 * depending on the gcc version.
 		 */
-		asm ("eor\t%0, %1, %1, ror #16" : "=r" (t) : "r" (x));
+		__asm__ ("eor\t%0, %1, %1, ror #16" : "=r" (t) : "r" (x));
 	} else
 #endif
 		t = x ^ ((x << 16) | (x >> 16)); /* eor r1,r0,r0,ror #16 */
diff --git a/include/asm-i386/byteorder.h b/include/asm-i386/byteorder.h
index a45470a..4ead40b 100644
--- a/include/asm-i386/byteorder.h
+++ b/include/asm-i386/byteorder.h
@@ -32,13 +32,13 @@ static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 val)
 	} v;
 	v.u = val;
 #ifdef CONFIG_X86_BSWAP
-	asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1" 
-	    : "=r" (v.s.a), "=r" (v.s.b) 
-	    : "0" (v.s.a), "1" (v.s.b)); 
+	__asm__("bswapl %0 ; bswapl %1 ; xchgl %0,%1" 
+	        : "=r" (v.s.a), "=r" (v.s.b) 
+	        : "0" (v.s.a), "1" (v.s.b)); 
 #else
    v.s.a = ___arch__swab32(v.s.a); 
 	v.s.b = ___arch__swab32(v.s.b); 
-	asm("xchgl %0,%1" : "=r" (v.s.a), "=r" (v.s.b) : "0" (v.s.a), "1" (v.s.b));
+	__asm__("xchgl %0,%1" : "=r" (v.s.a), "=r" (v.s.b) : "0" (v.s.a), "1" (v.s.b));
 #endif
 	return v.u;	
 } 
diff --git a/include/asm-s390/byteorder.h b/include/asm-s390/byteorder.h
index 1fe2492..07230f6 100644
--- a/include/asm-s390/byteorder.h
+++ b/include/asm-s390/byteorder.h
@@ -18,7 +18,7 @@ static inline __u64 ___arch__swab64p(const __u64 *x)
 {
 	__u64 result;
 
-	asm volatile("lrvg %0,%1" : "=d" (result) : "m" (*x));
+	__asm__ __volatile__("lrvg %0,%1" : "=d" (result) : "m" (*x));
 	return result;
 }
 
@@ -26,7 +26,7 @@ static inline __u64 ___arch__swab64(__u64 x)
 {
 	__u64 result;
 
-	asm volatile("lrvgr %0,%1" : "=d" (result) : "d" (x));
+	__asm__ __volatile__("lrvgr %0,%1" : "=d" (result) : "d" (x));
 	return result;
 }
 
@@ -40,7 +40,7 @@ static inline __u32 ___arch__swab32p(const __u32 *x)
 {
 	__u32 result;
 	
-	asm volatile(
+	__asm__ __volatile__(
 #ifndef __s390x__
 		"	icm	%0,8,3(%1)\n"
 		"	icm	%0,4,2(%1)\n"
@@ -61,7 +61,7 @@ static inline __u32 ___arch__swab32(__u32 x)
 #else /* __s390x__ */
 	__u32 result;
 	
-	asm volatile("lrvr  %0,%1" : "=d" (result) : "d" (x));
+	__asm__ __volatile__("lrvr  %0,%1" : "=d" (result) : "d" (x));
 	return result;
 #endif /* __s390x__ */
 }
@@ -75,7 +75,7 @@ static __inline__ __u16 ___arch__swab16p(const __u16 *x)
 {
 	__u16 result;
 	
-	asm volatile(
+	__asm__ __volatile__(
 #ifndef __s390x__
 		"	icm	%0,2,1(%1)\n"
 		"	ic	%0,0(%1)\n"

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Arjan van de Ven]

On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> This changes asm() to __asm__() and volatile to __volatile__ so that these
> headers can be used with gcc's -std=c99.

hmm but the kernel doesn't use -std=c99...

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Arnd Bergmann]

On Monday 18 June 2007, Arjan van de Ven wrote:
> 
> On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> > This changes asm() to __asm__() and volatile to __volatile__ so that these
> > headers can be used with gcc's -std=c99.
> 
> hmm but the kernel doesn't use -std=c99...

The byteorder headers are exported to user space through
include/asm-generic/Kbuild.asm, and they are used by a number
of other exported headers, so they should work with any
gcc flags that a user might want to use.

	Arnd <><

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Mike Frysinger]

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

On Sunday 17 June 2007, Arnd Bergmann wrote:
> On Monday 18 June 2007, Arjan van de Ven wrote:
> > On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> > > This changes asm() to __asm__() and volatile to __volatile__ so that
> > > these headers can be used with gcc's -std=c99.
> >
> > hmm but the kernel doesn't use -std=c99...
>
> The byteorder headers are exported to user space through
> include/asm-generic/Kbuild.asm, and they are used by a number
> of other exported headers, so they should work with any
> gcc flags that a user might want to use.

yeah, sorry, i should have mentioned the point here was that these are 
exported headers
-mike

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

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[David Woodhouse]

On Mon, 2007-06-18 at 01:24 +0200, Arnd Bergmann wrote:
> On Monday 18 June 2007, Arjan van de Ven wrote:
> > 
> > On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> > > This changes asm() to __asm__() and volatile to __volatile__ so that these
> > > headers can be used with gcc's -std=c99.
> > 
> > hmm but the kernel doesn't use -std=c99...
> 
> The byteorder headers are exported to user space through
> include/asm-generic/Kbuild.asm, and they are used by a number
> of other exported headers, so they should work with any
> gcc flags that a user might want to use. 

Even those headers which are exported are _still_ kernel headers¹. The
'caveat emptor' principle still applies to them, and we don't have to be
_that_ anal about it. GNU extensions (and proper C types, for that
matter) should be acceptable, surely?

-- 
dwmw2

¹ well, except perhaps for the very few headers which get included 
  directly by glibc's headers, but aren't we still pretending that
  doesn't happen?

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 01:24:24AM +0200, Arnd Bergmann wrote:
> On Monday 18 June 2007, Arjan van de Ven wrote:
> > 
> > On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> > > This changes asm() to __asm__() and volatile to __volatile__ so that these
> > > headers can be used with gcc's -std=c99.
> > 
> > hmm but the kernel doesn't use -std=c99...
> 
> The byteorder headers are exported to user space through
> include/asm-generic/Kbuild.asm, and they are used by a number
> of other exported headers, so they should work with any
> gcc flags that a user might want to use.

No, they should not be exported and the headers using them
should be fixed to not require this.  Userspace has it's own
endianess handling already.

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Mike Frysinger]

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

On Monday 18 June 2007, Christoph Hellwig wrote:
> On Mon, Jun 18, 2007 at 01:24:24AM +0200, Arnd Bergmann wrote:
> > On Monday 18 June 2007, Arjan van de Ven wrote:
> > > On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
> > > > This changes asm() to __asm__() and volatile to __volatile__ so that
> > > > these headers can be used with gcc's -std=c99.
> > >
> > > hmm but the kernel doesn't use -std=c99...
> >
> > The byteorder headers are exported to user space through
> > include/asm-generic/Kbuild.asm, and they are used by a number
> > of other exported headers, so they should work with any
> > gcc flags that a user might want to use.
>
> No, they should not be exported and the headers using them
> should be fixed to not require this.  Userspace has it's own
> endianess handling already.

user applications arent pulling these things in themselves ... you have to 
also think of the cascading of header includes ... asm/byteorder.h gets 
pulled in by many other things

if we want to scrub the userspace headers so that asm/byteorder.h isnt even 
installed, that works for me as well, however i wouldnt discount the patch i 
proposed on this alone ... the headers are inconsistent between using asm and 
__asm__ and if anything, my patch makes them consistent
-mike

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

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Christoph Hellwig]

On Sun, Jun 17, 2007 at 06:33:28PM -0400, Mike Frysinger wrote:
> This changes asm() to __asm__() and volatile to __volatile__ so that these
> headers can be used with gcc's -std=c99.

We should not allow inline assemly in the exported part of userspace headers
at all.  These headers must only include defintions for the kernel <-> user
ABI, and should not include code at all.

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 07:34:50PM +0100, Christoph Hellwig wrote:
> On Sun, Jun 17, 2007 at 06:33:28PM -0400, Mike Frysinger wrote:
> > This changes asm() to __asm__() and volatile to __volatile__ so that these
> > headers can be used with gcc's -std=c99.
> 
> We should not allow inline assemly in the exported part of userspace headers
> at all.  These headers must only include defintions for the kernel <-> user
> ABI, and should not include code at all.

this should have been a reply to the hdrcheck thread, but given the
cc list of this one is a superset of that I hope everyone can find it anyway.

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Sam Ravnborg]

On Mon, Jun 18, 2007 at 07:34:50PM +0100, Christoph Hellwig wrote:
> On Sun, Jun 17, 2007 at 06:33:28PM -0400, Mike Frysinger wrote:
> > This changes asm() to __asm__() and volatile to __volatile__ so that these
> > headers can be used with gcc's -std=c99.
> 
> We should not allow inline assemly in the exported part of userspace headers
> at all.  These headers must only include defintions for the kernel <-> user
> ABI, and should not include code at all.
Do you imply that if we see asm or __asm__ in user space headers we ougth
to warn about it?
Seems at least sensible to me but if we introduce such a check we should
kill all offenders first - which Mike's patches seems to trigger for some part.

	Sam

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 09:00:09PM +0200, Sam Ravnborg wrote:
> Do you imply that if we see asm or __asm__ in user space headers we ougth
> to warn about it?
> Seems at least sensible to me but if we introduce such a check we should
> kill all offenders first - which Mike's patches seems to trigger for some part.

Yes, we should kill the offenders first.  Some of Mike's patches are moving
nicely in that directions.  The biggest problem from my POV is byteawap.h
that gets dragged in in various places right now and needs to be sorted out.

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 08:27:15AM -0600, Robert Hancock wrote:
> If we expect userspace apps to include them, then I would vote for no, 
> not for anything outside of #ifdef __KERNEL__ in exported headers. Keep 
> in mind also that C++ apps may need to include these as well and those 
> extensions don't always play well in C++ mode. (Last instance I ran into 
> was the ioctl argument checking macros _IOR, _IOW, etc. that create 
> non-compiling code if you use them in a C++ program.)

There's absolutely no reason to obfucate kernel headers for usage in
C++ source files.

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[H. Peter Anvin]

Robert Hancock wrote:
> 
> If we expect userspace apps to include them, then I would vote for no,
> not for anything outside of #ifdef __KERNEL__ in exported headers. Keep
> in mind also that C++ apps may need to include these as well and those
> extensions don't always play well in C++ mode. (Last instance I ran into
> was the ioctl argument checking macros _IOR, _IOW, etc. that create
> non-compiling code if you use them in a C++ program.)
> 

Some of the actual ioctl macros generate silently wrong code if you use
them in a C program.

	-hpa

Re: [patch] use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

[Robert Hancock]

David Woodhouse wrote:
> On Mon, 2007-06-18 at 01:24 +0200, Arnd Bergmann wrote:
>> On Monday 18 June 2007, Arjan van de Ven wrote:
>>> On Sun, 2007-06-17 at 18:33 -0400, Mike Frysinger wrote:
>>>> This changes asm() to __asm__() and volatile to __volatile__ so that these
>>>> headers can be used with gcc's -std=c99.
>>> hmm but the kernel doesn't use -std=c99...
>> The byteorder headers are exported to user space through
>> include/asm-generic/Kbuild.asm, and they are used by a number
>> of other exported headers, so they should work with any
>> gcc flags that a user might want to use. 
> 
> Even those headers which are exported are _still_ kernel headers¹. The
> 'caveat emptor' principle still applies to them, and we don't have to be
> _that_ anal about it. GNU extensions (and proper C types, for that
> matter) should be acceptable, surely?

If we expect userspace apps to include them, then I would vote for no, 
not for anything outside of #ifdef __KERNEL__ in exported headers. Keep 
in mind also that C++ apps may need to include these as well and those 
extensions don't always play well in C++ mode. (Last instance I ran into 
was the ioctl argument checking macros _IOR, _IOW, etc. that create 
non-compiling code if you use them in a C++ program.)

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/