2007-06-17 22:33:37

by Mike Frysinger

[permalink] [raw]
Subject: [patch] 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]>
---
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"


2007-06-17 22:54:17

by Arjan van de Ven

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

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


2007-06-17 23:25:03

by Arnd Bergmann

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

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

2007-06-17 23:55:38

by Mike Frysinger

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

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


Attachments:
(No filename) (657.00 B)
signature.asc (827.00 B)
This is a digitally signed message part.
Download all attachments

2007-06-18 08:51:40

by David Woodhouse

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

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?


2007-06-18 14:27:37

by Robert Hancock

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

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 [email protected]
Home Page: http://www.roberthancock.com/

2007-06-18 18:10:08

by H. Peter Anvin

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

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

2007-06-18 18:11:31

by Christoph Hellwig

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

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.

2007-06-18 18:12:21

by Christoph Hellwig

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

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.

2007-06-18 18:21:34

by Mike Frysinger

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

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


Attachments:
(No filename) (1.26 kB)
signature.asc (827.00 B)
This is a digitally signed message part.
Download all attachments

2007-06-18 18:35:07

by Christoph Hellwig

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

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.

2007-06-18 18:39:42

by Christoph Hellwig

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

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.

2007-06-18 18:59:24

by Sam Ravnborg

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

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

2007-06-19 04:20:08

by Christoph Hellwig

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

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.