[PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

Use __asm__/__inline__ rather than asm/inline for all the functions
exported to userspace.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/x86/include/asm/byteorder.h |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/byteorder.h b/arch/x86/include/asm/byteorder.h
index e02ae2d..16f7c01 100644
--- a/arch/x86/include/asm/byteorder.h
+++ b/arch/x86/include/asm/byteorder.h
@@ -8,12 +8,12 @@
 
 #ifdef __i386__
 
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
 #ifdef CONFIG_X86_BSWAP
-	asm("bswap %0" : "=r" (x) : "0" (x));
+	__asm__("bswap %0" : "=r" (x) : "0" (x));
 #else
-	asm("xchgb %b0,%h0\n\t"	/* swap lower bytes	*/
+	__asm__("xchgb %b0,%h0\n\t"	/* swap lower bytes	*/
 	    "rorl $16,%0\n\t"	/* swap words		*/
 	    "xchgb %b0,%h0"	/* swap higher bytes	*/
 	    : "=q" (x)
@@ -22,7 +22,7 @@ static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
 	return x;
 }
 
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 val)
 {
 	union {
 		struct {
@@ -33,13 +33,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"
+	__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"
+	__asm__("xchgl %0,%1"
 	    : "=r" (v.s.a), "=r" (v.s.b)
 	    : "0" (v.s.a), "1" (v.s.b));
 #endif
@@ -48,17 +48,17 @@ static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
 
 #else /* __i386__ */
 
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 x)
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 x)
 {
-	asm("bswapq %0"
+	__asm__("bswapq %0"
 	    : "=r" (x)
 	    : "0" (x));
 	return x;
 }
 
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
-	asm("bswapl %0"
+	__asm__("bswapl %0"
 	    : "=r" (x)
 	    : "0" (x));
 	return x;
-- 
1.6.0.6

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Sam Ravnborg]

On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> Use __asm__/__inline__ rather than asm/inline for all the functions
> exported to userspace.

I will imagine that we will see cleanup patches converting
these back to inline/asm.
How about doing this conversion as part of the headers_install.pl
script so we know it is always correct?

Then we can keep the familiar inline/asm in the kernel headers
and always use the correct __asm__, __inline__ version for our
exported headers.

	Sam

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 02:12:08 Sam Ravnborg wrote:
> On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> > Use __asm__/__inline__ rather than asm/inline for all the functions
> > exported to userspace.
>
> I will imagine that we will see cleanup patches converting
> these back to inline/asm.
> How about doing this conversion as part of the headers_install.pl
> script so we know it is always correct?
>
> Then we can keep the familiar inline/asm in the kernel headers
> and always use the correct __asm__, __inline__ version for our
> exported headers.

yeah, i think these kind of "fixes" went in between 2.6.27 and 2.6.28 because 
there's a lot more hits on "inline" than there were previously ...
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> > Use __asm__/__inline__ rather than asm/inline for all the functions
> > exported to userspace.
> 
> I will imagine that we will see cleanup patches converting
> these back to inline/asm.
> How about doing this conversion as part of the headers_install.pl
> script so we know it is always correct?
> 
> Then we can keep the familiar inline/asm in the kernel headers
> and always use the correct __asm__, __inline__ version for our
> exported headers.

or name it __asm__exported__ to make sure it isnt cleaned up away.

	Ingo

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 03:47:08 Ingo Molnar wrote:
> * Sam Ravnborg wrote:
> > On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> > > Use __asm__/__inline__ rather than asm/inline for all the functions
> > > exported to userspace.
> >
> > I will imagine that we will see cleanup patches converting
> > these back to inline/asm.
> > How about doing this conversion as part of the headers_install.pl
> > script so we know it is always correct?
> >
> > Then we can keep the familiar inline/asm in the kernel headers
> > and always use the correct __asm__, __inline__ version for our
> > exported headers.
>
> or name it __asm__exported__ to make sure it isnt cleaned up away.

i think we can try the automated approach first as that'll make things easier 
on everyone ... but if it doesnt work out, this sounds like a reasonable 
solution to keep things sane.
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Sam Ravnborg]

On Sat, Dec 27, 2008 at 09:47:08AM +0100, Ingo Molnar wrote:
> 
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> > > Use __asm__/__inline__ rather than asm/inline for all the functions
> > > exported to userspace.
> > 
> > I will imagine that we will see cleanup patches converting
> > these back to inline/asm.
> > How about doing this conversion as part of the headers_install.pl
> > script so we know it is always correct?
> > 
> > Then we can keep the familiar inline/asm in the kernel headers
> > and always use the correct __asm__, __inline__ version for our
> > exported headers.
> 
> or name it __asm__exported__ to make sure it isnt cleaned up away.

I wnet with the scripted conversion for now.
If that does not fly we can come back to this proposal.

What I like most with the auto conversion is that we avoid
adding yet another special rule about how to do stuff in exported headers.

	Sam

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Sam Ravnborg wrote:
> 
> I wnet with the scripted conversion for now.
> If that does not fly we can come back to this proposal.
> 
> What I like most with the auto conversion is that we avoid
> adding yet another special rule about how to do stuff in exported headers.
> 

Indeed, and being keyword conversion, it's independent of context, at
least as long as one doesn't have too many run-ins with weird uses of
the # and ## preprocessor operators, which are a *lot* easier to rule
out globally.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Sam Ravnborg]

On Sat, Dec 27, 2008 at 10:58:11AM -0800, H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> > 
> > I wnet with the scripted conversion for now.
> > If that does not fly we can come back to this proposal.
> > 
> > What I like most with the auto conversion is that we avoid
> > adding yet another special rule about how to do stuff in exported headers.
> > 
> 
> Indeed, and being keyword conversion, it's independent of context, at
> least as long as one doesn't have too many run-ins with weird uses of
> the # and ## preprocessor operators, which are a *lot* easier to rule
> out globally.

Speaking of what we want to use in exported headers.
What is the recommendation with respect to uint32_t and friends?
To my best knowledge they are banned in exported headers as they
are not part of the kernel namespace and I see few users too.
But is this something we should check for?

	Sam

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Sam Ravnborg wrote:
> On Sat, Dec 27, 2008 at 10:58:11AM -0800, H. Peter Anvin wrote:
>> Sam Ravnborg wrote:
>>> I wnet with the scripted conversion for now.
>>> If that does not fly we can come back to this proposal.
>>>
>>> What I like most with the auto conversion is that we avoid
>>> adding yet another special rule about how to do stuff in exported headers.
>>>
>> Indeed, and being keyword conversion, it's independent of context, at
>> least as long as one doesn't have too many run-ins with weird uses of
>> the # and ## preprocessor operators, which are a *lot* easier to rule
>> out globally.
> 
> Speaking of what we want to use in exported headers.
> What is the recommendation with respect to uint32_t and friends?
> To my best knowledge they are banned in exported headers as they
> are not part of the kernel namespace and I see few users too.
> But is this something we should check for?

I personally would not be upset if we auto-changed {su}{8,16,32,64},
[u]int_{8,16,32,64}_t and bool into the appropriate __{su}{8,16,32,64}
types and _Bool.  I think the upside is way bigger than the potential
downside.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 14:15:01 H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> > On Sat, Dec 27, 2008 at 10:58:11AM -0800, H. Peter Anvin wrote:
> >> Sam Ravnborg wrote:
> >>> I wnet with the scripted conversion for now.
> >>> If that does not fly we can come back to this proposal.
> >>>
> >>> What I like most with the auto conversion is that we avoid
> >>> adding yet another special rule about how to do stuff in exported
> >>> headers.
> >>
> >> Indeed, and being keyword conversion, it's independent of context, at
> >> least as long as one doesn't have too many run-ins with weird uses of
> >> the # and ## preprocessor operators, which are a *lot* easier to rule
> >> out globally.
> >
> > Speaking of what we want to use in exported headers.
> > What is the recommendation with respect to uint32_t and friends?
> > To my best knowledge they are banned in exported headers as they
> > are not part of the kernel namespace and I see few users too.
> > But is this something we should check for?
>
> I personally would not be upset if we auto-changed {su}{8,16,32,64},
> [u]int_{8,16,32,64}_t

{su}{8,16,32,64} doesnt matter too much to me vs {u,}int_t{8,16,32,64}_t.  as 
long as people stop using __{su}{8,16,32,64}.  using the latter though does 
mean headers will more likely be "just usable" w/out needing linux/types.h 
include.  but then people would be forced to include stdint.h or similar 
before a linux header ... and that sucks.

unless of course we start adding appropriate C library includes for 
!__KERNEL__ ... i'd love that personally

> and bool into the appropriate __{su}{8,16,32,64}
> types and _Bool.

i dont get your bool comment.  the "bool" type is already a standard type.  
there is no conversion needed.
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Mike Frysinger wrote:
> 
> {su}{8,16,32,64} doesnt matter too much to me vs {u,}int_t{8,16,32,64}_t.  as 
> long as people stop using __{su}{8,16,32,64}.  using the latter though does 
> mean headers will more likely be "just usable" w/out needing linux/types.h 
> include.  but then people would be forced to include stdint.h or similar 
> before a linux header ... and that sucks.
> 

That is a total non-starter.  This would mean that the C library itself
cannot use these headers without exporting additional symbols into the
namespace, *WHICH IT IS NOT ALLOWED TO DO*.

> unless of course we start adding appropriate C library includes for 
> !__KERNEL__ ... i'd love that personally
> 
>> and bool into the appropriate __{su}{8,16,32,64}
>> types and _Bool.
> 
> i dont get your bool comment.  the "bool" type is already a standard type.  
> there is no conversion needed.

Only if <stdbool.h> is included.  Again, see previous statement.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 14:23:19 H. Peter Anvin wrote:
> Mike Frysinger wrote:
> > {su}{8,16,32,64} doesnt matter too much to me vs {u,}int_t{8,16,32,64}_t.
> >  as long as people stop using __{su}{8,16,32,64}.  using the latter
> > though does mean headers will more likely be "just usable" w/out needing
> > linux/types.h include.  but then people would be forced to include
> > stdint.h or similar before a linux header ... and that sucks.
>
> That is a total non-starter.  This would mean that the C library itself
> cannot use these headers without exporting additional symbols into the
> namespace, *WHICH IT IS NOT ALLOWED TO DO*.

which is already happening today you mean.  grep the kernel headers and you'll 
see a ton of [u]intXX_t hits.

this logic though means that the kernel should not be defining any structures 
that the C library is defining (such as asm-generic/fcntl.h).  such structs 
should get renamed the same way as __[us]XX types.
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Mike Frysinger wrote:
> On Saturday 27 December 2008 14:23:19 H. Peter Anvin wrote:
>> Mike Frysinger wrote:
>>> {su}{8,16,32,64} doesnt matter too much to me vs {u,}int_t{8,16,32,64}_t.
>>>  as long as people stop using __{su}{8,16,32,64}.  using the latter
>>> though does mean headers will more likely be "just usable" w/out needing
>>> linux/types.h include.  but then people would be forced to include
>>> stdint.h or similar before a linux header ... and that sucks.
>> That is a total non-starter.  This would mean that the C library itself
>> cannot use these headers without exporting additional symbols into the
>> namespace, *WHICH IT IS NOT ALLOWED TO DO*.
> 
> which is already happening today you mean.  grep the kernel headers and you'll 
> see a ton of [u]intXX_t hits.

Now, keep in mind this is only true for headers exported to userspace.
But this is correct - which is the base of this conversation (Sam
suggesting that they should be warned about, and I suggested
auto-converting them.)

> this logic though means that the kernel should not be defining any structures 
> that the C library is defining (such as asm-generic/fcntl.h).  such structs 
> should get renamed the same way as __[us]XX types

This is also correct, at least for exported headers.  For
kernel-internal headers, it doesn't matter.  Unfortunately we do have at
least several cases of exported interfaces with globally visible names.

There is one other exception of note, which is a header file which can
only be included by the userspace *application*, using a nonportable
top-level include (either directly <linux/*> or indirectly via <sys/*>).
 In those cases we can be looser about at least structure names.  This
is common for ioctl structures.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 15:45:25 H. Peter Anvin wrote:
> Mike Frysinger wrote:
> > On Saturday 27 December 2008 14:23:19 H. Peter Anvin wrote:
> >> Mike Frysinger wrote:
> >>> {su}{8,16,32,64} doesnt matter too much to me vs
> >>> {u,}int_t{8,16,32,64}_t. as long as people stop using
> >>> __{su}{8,16,32,64}.  using the latter though does mean headers will
> >>> more likely be "just usable" w/out needing linux/types.h include.  but
> >>> then people would be forced to include stdint.h or similar before a
> >>> linux header ... and that sucks.
> >>
> >> That is a total non-starter.  This would mean that the C library itself
> >> cannot use these headers without exporting additional symbols into the
> >> namespace, *WHICH IT IS NOT ALLOWED TO DO*.
> >
> > which is already happening today you mean.  grep the kernel headers and
> > you'll see a ton of [u]intXX_t hits.
>
> Now, keep in mind this is only true for headers exported to userspace.
> But this is correct - which is the base of this conversation (Sam
> suggesting that they should be warned about, and I suggested
> auto-converting them.)

if the topic is for autoconverting the uintXX_t (and related) types to __uXX 
(and related), then that's easy to do i would think.  i can post a patch to do 
that.

> > this logic though means that the kernel should not be defining any
> > structures that the C library is defining (such as asm-generic/fcntl.h). 
> > such structs should get renamed the same way as __[us]XX types
>
> This is also correct, at least for exported headers.  For
> kernel-internal headers, it doesn't matter.  Unfortunately we do have at
> least several cases of exported interfaces with globally visible names.
>
> There is one other exception of note, which is a header file which can
> only be included by the userspace *application*, using a nonportable
> top-level include (either directly <linux/*> or indirectly via <sys/*>).
>  In those cases we can be looser about at least structure names.  This
> is common for ioctl structures.

the structures that go through ioctls rarely are an issue.  trouble often 
brews when dealing with POSIX functions that have a very similar (if not 
direct) version in the kernel interface that utilize structures.  i mention 
the fcntl structure because ive seen that with a couple of packages where the 
version in the kernel headers conflicts with the version in the C library.  or 
where the kernel ABI structure is not directly compatible with the POSIX C 
library structure ... the stat struct is an example of this where the C 
library has to know the layout of both and converts one to the other.

looking at asm-generic/fcntl.h, i'd propose this change as a template:
...
#ifdef __KERNEL__
# define kernel_flock flock
#endif
struct kernel_flock {
...
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Mike Frysinger wrote:
> 
> looking at asm-generic/fcntl.h, i'd propose this change as a template:
> ...
> #ifdef __KERNEL__
> # define kernel_flock flock
> #endif
> struct kernel_flock {
> ...
> -mike

It should be __kernel_flock or something like that.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Mike Frysinger wrote:
> ...
> #ifdef __KERNEL__
> # define kernel_flock flock
> #endif
> struct kernel_flock {
> ...

FWIW, a long time ago I proposed the __kabi_ prefix for these things; I
think it is a good thing to make userspace-visible structures stand out
even in the kernel code itself.

As such, I would suggest *not* adding the #define at all.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Mike Frysinger]

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

On Saturday 27 December 2008 16:09:52 H. Peter Anvin wrote:
> Mike Frysinger wrote:
> > ...
> > #ifdef __KERNEL__
> > # define kernel_flock flock
> > #endif
> > struct kernel_flock {
> > ...
>
> FWIW, a long time ago I proposed the __kabi_ prefix for these things; I
> think it is a good thing to make userspace-visible structures stand out
> even in the kernel code itself.
>
> As such, I would suggest *not* adding the #define at all.

the convention based on linux/types.h seems to be to use __kernel_ as the 
prefix, but it doesnt matter to me whether it's __kabi_ or __kernel_.  i'll 
start doing the latter though in the hopes it'll be less scary ;).
-mike

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

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Mike Frysinger wrote:
> On Saturday 27 December 2008 16:09:52 H. Peter Anvin wrote:
>> Mike Frysinger wrote:
>>> ...
>>> #ifdef __KERNEL__
>>> # define kernel_flock flock
>>> #endif
>>> struct kernel_flock {
>>> ...
>> FWIW, a long time ago I proposed the __kabi_ prefix for these things; I
>> think it is a good thing to make userspace-visible structures stand out
>> even in the kernel code itself.
>>
>> As such, I would suggest *not* adding the #define at all.
> 
> the convention based on linux/types.h seems to be to use __kernel_ as the 
> prefix, but it doesnt matter to me whether it's __kabi_ or __kernel_.  i'll 
> start doing the latter though in the hopes it'll be less scary ;).

Agreed, the actual prefix doesn't matter.  I suggested __kabi to be more
explicit about the fact that it is an exported ABI, but I think the main
thing is to not #define it out.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Sam Ravnborg]

On Sat, Dec 27, 2008 at 11:15:01AM -0800, H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> > On Sat, Dec 27, 2008 at 10:58:11AM -0800, H. Peter Anvin wrote:
> >> Sam Ravnborg wrote:
> >>> I wnet with the scripted conversion for now.
> >>> If that does not fly we can come back to this proposal.
> >>>
> >>> What I like most with the auto conversion is that we avoid
> >>> adding yet another special rule about how to do stuff in exported headers.
> >>>
> >> Indeed, and being keyword conversion, it's independent of context, at
> >> least as long as one doesn't have too many run-ins with weird uses of
> >> the # and ## preprocessor operators, which are a *lot* easier to rule
> >> out globally.
> > 
> > Speaking of what we want to use in exported headers.
> > What is the recommendation with respect to uint32_t and friends?
> > To my best knowledge they are banned in exported headers as they
> > are not part of the kernel namespace and I see few users too.
> > But is this something we should check for?
> 
> I personally would not be upset if we auto-changed {su}{8,16,32,64},
> [u]int_{8,16,32,64}_t and bool into the appropriate __{su}{8,16,32,64}
> types and _Bool.  I think the upside is way bigger than the potential
> downside.

I was only thinking of a warning so we could have this part cleaned up.
It is for the most part a trivial task to do for a volunteer.

I could also see some sporadic use of u_int32_t - dunno where it comes from.
But looks wrong to me.

	Sam

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Sam Ravnborg wrote:
> 
> I was only thinking of a warning so we could have this part cleaned up.
> It is for the most part a trivial task to do for a volunteer.
> 

Why do a warning and then have people hack it manually?  If we do it
automatically, we should be able to get rid of *another* special rule
for user-exported headers, and make the code look more similar again.

> I could also see some sporadic use of u_int32_t - dunno where it comes from.
> But looks wrong to me.

I think it's an old BSDism.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

[PATCH] kbuild: auto-convert size types in userspace headers

[Mike Frysinger]

Rather than constantly fixing up size type breakage in userspace headers,
auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
appropriate __uXX or __sXX type.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 scripts/headers_install.pl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
index c6ae405..697f02d 100644
--- a/scripts/headers_install.pl
+++ b/scripts/headers_install.pl
@@ -39,6 +39,9 @@ foreach my $file (@files) {
 		$line =~ s/(^|\s)(inline)\b/$1__$2__/g;
 		$line =~ s/(^|\s)(asm)\b(\s|[(]|$)/$1__$2__$3/g;
 		$line =~ s/(^|\s|[(])(volatile)\b(\s|[(]|$)/$1__$2__$3/g;
+		$line =~ s/\b([us](8|16|32|64))\b/__$1/g;
+		$line =~ s/\b(u_?int(8|16|32|64)_t)\b/__u$2/g;
+		$line =~ s/\b(int(8|16|32|64)_t)\b/__s$2/g;
 		printf OUTFILE "%s", $line;
 	}
 	close OUTFILE;
-- 
1.6.0.6

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[Sam Ravnborg]

On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
> Rather than constantly fixing up size type breakage in userspace headers,
> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
> appropriate __uXX or __sXX type.

Is this the right thing to do?
uintXX_t belongs to a namespace that the kernel should not use.

So being restrictive here would be better I think.

	Sam

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[Mike Frysinger]

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

On Monday 29 December 2008 09:03:56 Sam Ravnborg wrote:
> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
> > Rather than constantly fixing up size type breakage in userspace headers,
> > auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
> > appropriate __uXX or __sXX type.
>
> Is this the right thing to do?
> uintXX_t belongs to a namespace that the kernel should not use.

some headers are shared between projects and so use the uintXX_t form, so i'm 
not sure outright banning it is a nice answer for them
-mike

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

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[H. Peter Anvin]

Mike Frysinger wrote:
> On Monday 29 December 2008 09:03:56 Sam Ravnborg wrote:
>> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
>>> Rather than constantly fixing up size type breakage in userspace headers,
>>> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
>>> appropriate __uXX or __sXX type.
>> Is this the right thing to do?
>> uintXX_t belongs to a namespace that the kernel should not use.
> 
> some headers are shared between projects and so use the uintXX_t form, so i'm 
> not sure outright banning it is a nice answer for them

Arguably, the right answer is the opposite... the Linux u* and s* forms
feel much more like nonstandard legacy code to me.  They have the
advantage of brevity, however.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[Mike Frysinger]

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

On Monday 29 December 2008 15:36:41 H. Peter Anvin wrote:
> Mike Frysinger wrote:
> > On Monday 29 December 2008 09:03:56 Sam Ravnborg wrote:
> >> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
> >>> Rather than constantly fixing up size type breakage in userspace
> >>> headers, auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and
> >>> sXX to the appropriate __uXX or __sXX type.
> >>
> >> Is this the right thing to do?
> >> uintXX_t belongs to a namespace that the kernel should not use.
> >
> > some headers are shared between projects and so use the uintXX_t form, so
> > i'm not sure outright banning it is a nice answer for them
>
> Arguably, the right answer is the opposite... the Linux u* and s* forms
> feel much more like nonstandard legacy code to me.  They have the
> advantage of brevity, however.

my testing of this patch has revealed an issue along these lines.  building 
mplayer fails as it uses linux/dvb/audio.h.  this header does:
#ifdef __KERNEL__
#include <linux/types.h>
#else
#include <stdint.h>
#endif
typedef uint16_t audio_attributes_t;

but since the script has autoconverted uint16_t to __u16, and linux/types.h 
isnt being pulled in for userspace, we get a build failure.
-mike

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

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[H. Peter Anvin]

Sam Ravnborg wrote:
> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
>> Rather than constantly fixing up size type breakage in userspace headers,
>> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
>> appropriate __uXX or __sXX type.
> 
> Is this the right thing to do?
> uintXX_t belongs to a namespace that the kernel should not use.
> 
> So being restrictive here would be better I think.

That's the whole point right here.

All of these types can be -- and are -- used by the kernel internally,
but should only be exported as __uXX or __sXX.

This is the auto-conversion idea I brought up earlier.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[Sam Ravnborg]

On Mon, Dec 29, 2008 at 03:35:45PM -0800, H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> > On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
> >> Rather than constantly fixing up size type breakage in userspace headers,
> >> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
> >> appropriate __uXX or __sXX type.
> > 
> > Is this the right thing to do?
> > uintXX_t belongs to a namespace that the kernel should not use.
> > 
> > So being restrictive here would be better I think.
> 
> That's the whole point right here.
> 
> All of these types can be -- and are -- used by the kernel internally,
> but should only be exported as __uXX or __sXX.
> 
> This is the auto-conversion idea I brought up earlier.

So the patch from Mike has your "Acked-by"?

	Sam

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[H. Peter Anvin]

Sam Ravnborg wrote:
> On Mon, Dec 29, 2008 at 03:35:45PM -0800, H. Peter Anvin wrote:
>> Sam Ravnborg wrote:
>>> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
>>>> Rather than constantly fixing up size type breakage in userspace headers,
>>>> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
>>>> appropriate __uXX or __sXX type.
>>> Is this the right thing to do?
>>> uintXX_t belongs to a namespace that the kernel should not use.
>>>
>>> So being restrictive here would be better I think.
>> That's the whole point right here.
>>
>> All of these types can be -- and are -- used by the kernel internally,
>> but should only be exported as __uXX or __sXX.
>>
>> This is the auto-conversion idea I brought up earlier.
> 
> So the patch from Mike has your "Acked-by"?
> 

Acked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] kbuild: auto-convert size types in userspace headers

[Sam Ravnborg]

On Tue, Dec 30, 2008 at 09:42:47AM -0800, H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> > On Mon, Dec 29, 2008 at 03:35:45PM -0800, H. Peter Anvin wrote:
> >> Sam Ravnborg wrote:
> >>> On Mon, Dec 29, 2008 at 06:12:33AM -0500, Mike Frysinger wrote:
> >>>> Rather than constantly fixing up size type breakage in userspace headers,
> >>>> auto convert the types u_intXX_t, uintXX_t, intXX_t, uXX, and sXX to the
> >>>> appropriate __uXX or __sXX type.
> >>> Is this the right thing to do?
> >>> uintXX_t belongs to a namespace that the kernel should not use.
> >>>
> >>> So being restrictive here would be better I think.
> >> That's the whole point right here.
> >>
> >> All of these types can be -- and are -- used by the kernel internally,
> >> but should only be exported as __uXX or __sXX.
> >>
> >> This is the auto-conversion idea I brought up earlier.
> > 
> > So the patch from Mike has your "Acked-by"?
> > 
> 
> Acked-by: H. Peter Anvin <hpa@zytor.com>

Hi hpa.

Despite your ack I chickened out on this one.
We have enough crunch in the userspace headers for now,
so lets see where we end with this before doing any autoconversion.

	Sam

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[David Woodhouse]

On Sat, 2008-12-27 at 20:12 +0100, Sam Ravnborg wrote:
> 
> Speaking of what we want to use in exported headers.
> What is the recommendation with respect to uint32_t and friends?
> To my best knowledge they are banned in exported headers as they
> are not part of the kernel namespace and I see few users too.
> But is this something we should check for?

No, they're not banned. There are a few cases where they can't be used
and we have to use the kernels "speshul" __u32 types, because certain
headers get included by glibc and we can't "pollute" the namespace with
the C standard types. But in _most_ cases, using the standard C types is
just fine.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Ingo Molnar wrote:
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
>> On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
>>> Use __asm__/__inline__ rather than asm/inline for all the functions
>>> exported to userspace.
>> I will imagine that we will see cleanup patches converting
>> these back to inline/asm.
>> How about doing this conversion as part of the headers_install.pl
>> script so we know it is always correct?
>>
>> Then we can keep the familiar inline/asm in the kernel headers
>> and always use the correct __asm__, __inline__ version for our
>> exported headers.
> 
> or name it __asm__exported__ to make sure it isnt cleaned up away.
> 

Seems better to do it automatically.  We end up with cleaner-looking
code that way, too.

It doesn't solve the need for symbols in inline functions in exported
headers to have __ prefixes, though.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[Marcin Slusarz]

On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
> Use __asm__/__inline__ rather than asm/inline for all the functions
> exported to userspace.

Why? I can't find anything related in Documentation/.

Marcin

Re: [PATCH] x86 byteorder.h: use __asm__/__inline__ for userspace

[H. Peter Anvin]

Marcin Slusarz wrote:
> On Sat, Dec 27, 2008 at 01:50:04AM -0500, Mike Frysinger wrote:
>> Use __asm__/__inline__ rather than asm/inline for all the functions
>> exported to userspace.
> 
> Why? I can't find anything related in Documentation/.

Because "asm" and "inline" are unsafe keywords in arbitrary C code, as
may be encountered in userspace.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.