Skip to contentSkip to conversation

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

PATCH In review Mike Frysinger 6 participants 11 messages Jun 17, 2007 – Jun 19, 2007 s390

lore
Share & tools
lore

Mike Frysinger

author
#0 lore raw

PATCH 1/1 use __asm__ and __volatile__ in i386/arm/s390 byteorder.h

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 <[email protected]>

---

include/asm-arm/byteorder.h +1 -1
index e6f7fcd..39105dc 100644
@@ -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 */
include/asm-i386/byteorder.h +4 -4
index a45470a..4ead40b 100644
@@ -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;	
 }
include/asm-s390/byteorder.h +5 -5
index 1fe2492..07230f6 100644
@@ -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"
#1 lore raw
Quoted 2 lines · Mike Frysinger

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...

#2 lore raw
Quoted 6 lines · Arjan van de Ven
Quoted 2 lines · Mike Frysinger

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 <><
author
#3 lore raw
Quoted 11 lines · Arnd Bergmann
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · Mike Frysinger

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

#4 lore raw
In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · Mike Frysinger

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

Quoted 6 lines
Quoted 1 line

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?

Signature · 5 lines
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?
#5 lore raw
In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · Mike Frysinger

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

Quoted 6 lines
Quoted 1 line

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.

author
#6 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · Mike Frysinger

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...

Quoted 8 lines
Quoted 4 lines

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

#7 lore raw
In reply to Mike Frysinger
Quoted 2 lines · Mike Frysinger

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.

#8 lore raw
Quoted 7 lines · Christoph Hellwig
Quoted 2 lines · Mike Frysinger

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.

#9 lore raw
In reply to Christoph Hellwig
Quoted 7 lines · Christoph Hellwig
Quoted 2 lines · Mike Frysinger

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

#10 lore raw
Quoted 4 lines · Sam Ravnborg

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.