Use __asm__/__inline__ rather than asm/inline for all the functions
exported to userspace.
Signed-off-by: Mike Frysinger <[email protected]>
---
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
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
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
* Sam Ravnborg <[email protected]> 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
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
On Sat, Dec 27, 2008 at 09:47:08AM +0100, Ingo Molnar wrote:
>
> * Sam Ravnborg <[email protected]> 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
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.
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
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.
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
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
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.
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.
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
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.
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
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.
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.
Ingo Molnar wrote:
> * Sam Ravnborg <[email protected]> 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.
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
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.
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 <[email protected]>
---
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
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
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
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.
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
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.
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
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.
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
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 <[email protected]>
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
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
[email protected] Intel Corporation
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 <[email protected]>
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