Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Miklos Szeredi
On Wed, Apr 17, 2013 at 9:45 PM, Colin Cross  wrote:
> This fixes my build issue, and doesn't introduce a new make
> headers_check warning so you can add my Tested-by: Colin Cross
> 

Thanks for testing.

Linus,  can you please pull this single fix from:

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-linus

Thanks,
Miklos


Miklos Szeredi (1):
  fuse: fix type definitions in uapi header

---
 include/uapi/linux/fuse.h |  436 ++---
 1 file changed, 216 insertions(+), 220 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Colin Cross
On Wed, Apr 17, 2013 at 2:57 AM, Miklos Szeredi  wrote:
> On Tue, Apr 16, 2013 at 01:00:37PM -0700, Colin Cross wrote:
>
>> Every other uapi header includes linux/types.h to get its type
>> definitions, and fuse.h should do the same when compiling for
>> userspace targeting linux
>
> Not necessarily.
>
> Here's a patch (largish but only search) that should fix all the 
> issues
> and actually removes a hack instead of adding more.
>
> Thanks,
> Miklos
> 
>
> From: Miklos Szeredi 
> Subject: fuse: fix type definitions in uapi header
>
> Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
> header with one used in library) added #ifdef __linux__ around
> defines if it is not set.  The kernel build is self-contained and
> can be built on non-Linux toolchains.  After the mentioned commit
> builds on non-Linux toolchains will try to include stdint.h and
> fail due to -nostdinc, and then fail with a bunch of undefined
> type errors.
>
> Fix by checking for __KERNEL__ instead of __linux__ and using the standard int
> types instead of the linux specific ones.
>
> Reported-by: Arve Hjønnevåg 
> Reported-by: Colin Cross 
> Signed-off-by: Miklos Szeredi 

This fixes my build issue, and doesn't introduce a new make
headers_check warning so you can add my Tested-by: Colin Cross


I still like my patch better though.  This patch results in different
typedefs being used when compiling in userspace vs. kernel, so you're
trusting that the host stdint.h matches the kernel's definitions.  In
practice I don't see how they would differ, so I guess its up to you.
If it were up to me, I would use linux/types.h for kernel and linux
userspace, and maybe replace the #defines with typedefs for non-linux
userspace builds.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 01:00:37PM -0700, Colin Cross wrote:

> Every other uapi header includes linux/types.h to get its type
> definitions, and fuse.h should do the same when compiling for
> userspace targeting linux

Not necessarily.

Here's a patch (largish but only search) that should fix all the issues
and actually removes a hack instead of adding more.

Thanks,
Miklos


From: Miklos Szeredi 
Subject: fuse: fix type definitions in uapi header

Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
header with one used in library) added #ifdef __linux__ around
defines if it is not set.  The kernel build is self-contained and
can be built on non-Linux toolchains.  After the mentioned commit
builds on non-Linux toolchains will try to include stdint.h and
fail due to -nostdinc, and then fail with a bunch of undefined
type errors.

Fix by checking for __KERNEL__ instead of __linux__ and using the standard int
types instead of the linux specific ones.

Reported-by: Arve Hjønnevåg 
Reported-by: Colin Cross 
Signed-off-by: Miklos Szeredi 
---
 include/uapi/linux/fuse.h |  436 ++
 1 file changed, 216 insertions(+), 220 deletions(-)

--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -95,15 +95,10 @@
 #ifndef _LINUX_FUSE_H
 #define _LINUX_FUSE_H
 
-#ifdef __linux__
+#ifdef __KERNEL__
 #include 
 #else
 #include 
-#define __u64 uint64_t
-#define __s64 int64_t
-#define __u32 uint32_t
-#define __s32 int32_t
-#define __u16 uint16_t
 #endif
 
 /*
@@ -139,42 +134,42 @@
userspace works under 64bit kernels */
 
 struct fuse_attr {
-   __u64   ino;
-   __u64   size;
-   __u64   blocks;
-   __u64   atime;
-   __u64   mtime;
-   __u64   ctime;
-   __u32   atimensec;
-   __u32   mtimensec;
-   __u32   ctimensec;
-   __u32   mode;
-   __u32   nlink;
-   __u32   uid;
-   __u32   gid;
-   __u32   rdev;
-   __u32   blksize;
-   __u32   padding;
+   uint64_tino;
+   uint64_tsize;
+   uint64_tblocks;
+   uint64_tatime;
+   uint64_tmtime;
+   uint64_tctime;
+   uint32_tatimensec;
+   uint32_tmtimensec;
+   uint32_tctimensec;
+   uint32_tmode;
+   uint32_tnlink;
+   uint32_tuid;
+   uint32_tgid;
+   uint32_trdev;
+   uint32_tblksize;
+   uint32_tpadding;
 };
 
 struct fuse_kstatfs {
-   __u64   blocks;
-   __u64   bfree;
-   __u64   bavail;
-   __u64   files;
-   __u64   ffree;
-   __u32   bsize;
-   __u32   namelen;
-   __u32   frsize;
-   __u32   padding;
-   __u32   spare[6];
+   uint64_tblocks;
+   uint64_tbfree;
+   uint64_tbavail;
+   uint64_tfiles;
+   uint64_tffree;
+   uint32_tbsize;
+   uint32_tnamelen;
+   uint32_tfrsize;
+   uint32_tpadding;
+   uint32_tspare[6];
 };
 
 struct fuse_file_lock {
-   __u64   start;
-   __u64   end;
-   __u32   type;
-   __u32   pid; /* tgid */
+   uint64_tstart;
+   uint64_tend;
+   uint32_ttype;
+   uint32_tpid; /* tgid */
 };
 
 /**
@@ -364,143 +359,143 @@ enum fuse_notify_code {
 #define FUSE_COMPAT_ENTRY_OUT_SIZE 120
 
 struct fuse_entry_out {
-   __u64   nodeid; /* Inode ID */
-   __u64   generation; /* Inode generation: nodeid:gen must
-  be unique for the fs's lifetime */
-   __u64   entry_valid;/* Cache timeout for the name */
-   __u64   attr_valid; /* Cache timeout for the attributes */
-   __u32   entry_valid_nsec;
-   __u32   attr_valid_nsec;
+   uint64_tnodeid; /* Inode ID */
+   uint64_tgeneration; /* Inode generation: nodeid:gen must
+  be unique for the fs's lifetime */
+   uint64_tentry_valid;/* Cache timeout for the name */
+   uint64_tattr_valid; /* Cache timeout for the attributes */
+   uint32_tentry_valid_nsec;
+   uint32_tattr_valid_nsec;
struct fuse_attr attr;
 };
 
 struct fuse_forget_in {
-   __u64   nlookup;
+   uint64_tnlookup;
 };
 
 struct fuse_forget_one {
-   __u64   nodeid;
-   __u64   nlookup;
+   uint64_tnodeid;
+   uint64_tnlookup;
 };
 
 struct fuse_batch_forget_in {
-   __u32   count;
-   __u32   dummy;
+   uint32_tcount;
+   uint32_tdummy;
 };
 
 struct fuse_getattr_in {
-   __u32   getattr_flags;
-   __u32   dummy;
-   __u64   fh;
+   uint32_tgetattr_flags;
+   uint32_tdummy;
+   uint64_tfh;
 };
 
 #define FUSE_COMPAT_ATTR_OUT_SIZE 96
 
 struct fuse_attr_out {
- 

Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 01:00:37PM -0700, Colin Cross wrote:

 Every other uapi header includes linux/types.h to get its type
 definitions, and fuse.h should do the same when compiling for
 userspace targeting linux

Not necessarily.

Here's a patch (largish but only searchreplace) that should fix all the issues
and actually removes a hack instead of adding more.

Thanks,
Miklos


From: Miklos Szeredi mszer...@suse.cz
Subject: fuse: fix type definitions in uapi header

Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
header with one used in library) added #ifdef __linux__ around
defines if it is not set.  The kernel build is self-contained and
can be built on non-Linux toolchains.  After the mentioned commit
builds on non-Linux toolchains will try to include stdint.h and
fail due to -nostdinc, and then fail with a bunch of undefined
type errors.

Fix by checking for __KERNEL__ instead of __linux__ and using the standard int
types instead of the linux specific ones.

Reported-by: Arve Hjønnevåg a...@android.com
Reported-by: Colin Cross ccr...@android.com
Signed-off-by: Miklos Szeredi mszer...@suse.cz
---
 include/uapi/linux/fuse.h |  436 ++
 1 file changed, 216 insertions(+), 220 deletions(-)

--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -95,15 +95,10 @@
 #ifndef _LINUX_FUSE_H
 #define _LINUX_FUSE_H
 
-#ifdef __linux__
+#ifdef __KERNEL__
 #include linux/types.h
 #else
 #include stdint.h
-#define __u64 uint64_t
-#define __s64 int64_t
-#define __u32 uint32_t
-#define __s32 int32_t
-#define __u16 uint16_t
 #endif
 
 /*
@@ -139,42 +134,42 @@
userspace works under 64bit kernels */
 
 struct fuse_attr {
-   __u64   ino;
-   __u64   size;
-   __u64   blocks;
-   __u64   atime;
-   __u64   mtime;
-   __u64   ctime;
-   __u32   atimensec;
-   __u32   mtimensec;
-   __u32   ctimensec;
-   __u32   mode;
-   __u32   nlink;
-   __u32   uid;
-   __u32   gid;
-   __u32   rdev;
-   __u32   blksize;
-   __u32   padding;
+   uint64_tino;
+   uint64_tsize;
+   uint64_tblocks;
+   uint64_tatime;
+   uint64_tmtime;
+   uint64_tctime;
+   uint32_tatimensec;
+   uint32_tmtimensec;
+   uint32_tctimensec;
+   uint32_tmode;
+   uint32_tnlink;
+   uint32_tuid;
+   uint32_tgid;
+   uint32_trdev;
+   uint32_tblksize;
+   uint32_tpadding;
 };
 
 struct fuse_kstatfs {
-   __u64   blocks;
-   __u64   bfree;
-   __u64   bavail;
-   __u64   files;
-   __u64   ffree;
-   __u32   bsize;
-   __u32   namelen;
-   __u32   frsize;
-   __u32   padding;
-   __u32   spare[6];
+   uint64_tblocks;
+   uint64_tbfree;
+   uint64_tbavail;
+   uint64_tfiles;
+   uint64_tffree;
+   uint32_tbsize;
+   uint32_tnamelen;
+   uint32_tfrsize;
+   uint32_tpadding;
+   uint32_tspare[6];
 };
 
 struct fuse_file_lock {
-   __u64   start;
-   __u64   end;
-   __u32   type;
-   __u32   pid; /* tgid */
+   uint64_tstart;
+   uint64_tend;
+   uint32_ttype;
+   uint32_tpid; /* tgid */
 };
 
 /**
@@ -364,143 +359,143 @@ enum fuse_notify_code {
 #define FUSE_COMPAT_ENTRY_OUT_SIZE 120
 
 struct fuse_entry_out {
-   __u64   nodeid; /* Inode ID */
-   __u64   generation; /* Inode generation: nodeid:gen must
-  be unique for the fs's lifetime */
-   __u64   entry_valid;/* Cache timeout for the name */
-   __u64   attr_valid; /* Cache timeout for the attributes */
-   __u32   entry_valid_nsec;
-   __u32   attr_valid_nsec;
+   uint64_tnodeid; /* Inode ID */
+   uint64_tgeneration; /* Inode generation: nodeid:gen must
+  be unique for the fs's lifetime */
+   uint64_tentry_valid;/* Cache timeout for the name */
+   uint64_tattr_valid; /* Cache timeout for the attributes */
+   uint32_tentry_valid_nsec;
+   uint32_tattr_valid_nsec;
struct fuse_attr attr;
 };
 
 struct fuse_forget_in {
-   __u64   nlookup;
+   uint64_tnlookup;
 };
 
 struct fuse_forget_one {
-   __u64   nodeid;
-   __u64   nlookup;
+   uint64_tnodeid;
+   uint64_tnlookup;
 };
 
 struct fuse_batch_forget_in {
-   __u32   count;
-   __u32   dummy;
+   uint32_tcount;
+   uint32_tdummy;
 };
 
 struct fuse_getattr_in {
-   __u32   getattr_flags;
-   __u32   dummy;
-   __u64   fh;
+   uint32_tgetattr_flags;
+   uint32_tdummy;
+   

Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Colin Cross
On Wed, Apr 17, 2013 at 2:57 AM, Miklos Szeredi mik...@szeredi.hu wrote:
 On Tue, Apr 16, 2013 at 01:00:37PM -0700, Colin Cross wrote:

 Every other uapi header includes linux/types.h to get its type
 definitions, and fuse.h should do the same when compiling for
 userspace targeting linux

 Not necessarily.

 Here's a patch (largish but only searchreplace) that should fix all the 
 issues
 and actually removes a hack instead of adding more.

 Thanks,
 Miklos
 

 From: Miklos Szeredi mszer...@suse.cz
 Subject: fuse: fix type definitions in uapi header

 Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
 header with one used in library) added #ifdef __linux__ around
 defines if it is not set.  The kernel build is self-contained and
 can be built on non-Linux toolchains.  After the mentioned commit
 builds on non-Linux toolchains will try to include stdint.h and
 fail due to -nostdinc, and then fail with a bunch of undefined
 type errors.

 Fix by checking for __KERNEL__ instead of __linux__ and using the standard int
 types instead of the linux specific ones.

 Reported-by: Arve Hjønnevåg a...@android.com
 Reported-by: Colin Cross ccr...@android.com
 Signed-off-by: Miklos Szeredi mszer...@suse.cz

This fixes my build issue, and doesn't introduce a new make
headers_check warning so you can add my Tested-by: Colin Cross
ccr...@android.com

I still like my patch better though.  This patch results in different
typedefs being used when compiling in userspace vs. kernel, so you're
trusting that the host stdint.h matches the kernel's definitions.  In
practice I don't see how they would differ, so I guess its up to you.
If it were up to me, I would use linux/types.h for kernel and linux
userspace, and maybe replace the #defines with typedefs for non-linux
userspace builds.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-17 Thread Miklos Szeredi
On Wed, Apr 17, 2013 at 9:45 PM, Colin Cross ccr...@android.com wrote:
 This fixes my build issue, and doesn't introduce a new make
 headers_check warning so you can add my Tested-by: Colin Cross
 ccr...@android.com

Thanks for testing.

Linus,  can you please pull this single fix from:

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-linus

Thanks,
Miklos


Miklos Szeredi (1):
  fuse: fix type definitions in uapi header

---
 include/uapi/linux/fuse.h |  436 ++---
 1 file changed, 216 insertions(+), 220 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Colin Cross
On Tue, Apr 16, 2013 at 12:11 PM, Miklos Szeredi  wrote:
> On Tue, Apr 16, 2013 at 8:29 PM, Colin Cross  wrote:
>> Dropping __linux__ causes a make headers_check warning, which the
>> kbuild test robot reported this morning:
>> usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
>> #include 
>> Using my patch without modification does not cause that warning.
>>
>> linux/types.h defines the types that are used to communicate between
>> the kernel and userspace.  Redefining those in each header makes no
>> sense, and will probably cause redefined types warnings if you compile
>> a userspace file that includes fuse.h and another uapi header that
>> properly includes linux/types.h.
>
>  is linux specific while the fuse API is cross
> platform.  So the userspace header obviously has to use some cross
> platform type definitions.  Making the type definitions depend on
> __linux__ is probably just as unreliable in userspace as it is in the
> kernel, so why complicate things with that?

I'm not sure what you mean by __linux__ being unreliable, it is set by
any toolchain that is compield with linux as its target.  It should be
very reliable in userspace, its only unreliable in the kernel because
the kernel doesn't rely on any specific os target in the toolchain, it
is self contained.

With your change, compiling a program that includes linux/fuse.h and
linux/icmp.h against the headers installed by make headers_installed
results in some strange preprocessor results.  The lines in
int-ll64.h, which is what normally defines the userspace types, are:
typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short __u16;

typedef __signed__ int __s32;
typedef unsigned int __u32;

After preprocessing with your fuse.h included before icmp.h (which
includes linux/types.h, which eventually includes
asm-generic/int-ll64.h):
typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short uint16_t;

typedef __signed__ int int32_t;
typedef unsigned int uint32_t;

The first three lines are the kernel headers providing typedefs for
the types that are not used in fuse.h, and therefore not covered by
the #define hacks there.  The last three lines are a problem: now you
have redefined the stdint types to match the kernel's types, better
hope they match the host's stdint definitions or you'll get redefined
type warnings.

Every other uapi header includes linux/types.h to get its type
definitions, and fuse.h should do the same when compiling for
userspace targeting linux or when compiling the kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 8:29 PM, Colin Cross  wrote:
> Dropping __linux__ causes a make headers_check warning, which the
> kbuild test robot reported this morning:
> usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
> #include 
> Using my patch without modification does not cause that warning.
>
> linux/types.h defines the types that are used to communicate between
> the kernel and userspace.  Redefining those in each header makes no
> sense, and will probably cause redefined types warnings if you compile
> a userspace file that includes fuse.h and another uapi header that
> properly includes linux/types.h.

 is linux specific while the fuse API is cross
platform.  So the userspace header obviously has to use some cross
platform type definitions.  Making the type definitions depend on
__linux__ is probably just as unreliable in userspace as it is in the
kernel, so why complicate things with that?

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Colin Cross
On Tue, Apr 16, 2013 at 11:21 AM, Linus Torvalds
 wrote:
>> What I meant is IF  is included by userspace (it sure is
>> meant to be included and *is* included by libfuse and other stuff)
>> THEN using  instead of  is fine regardless of
>> whether __linux__ is defined or not.
>
> That's probably true. But the patch in question adds the __KERNEL__
> test, and *that* seems required.
>
> If you think that we should instead drop the __linux__ test, than yes,
> that part sounds fine. I thought that by "linux internal header" you
> meant the fuse.h file, but you seem to mean the indirectly included
> . That's fine.
>
> Linus

Dropping __linux__ causes a make headers_check warning, which the
kbuild test robot reported this morning:
usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
#include 
Using my patch without modification does not cause that warning.

linux/types.h defines the types that are used to communicate between
the kernel and userspace.  Redefining those in each header makes no
sense, and will probably cause redefined types warnings if you compile
a userspace file that includes fuse.h and another uapi header that
properly includes linux/types.h.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Linus Torvalds
> What I meant is IF  is included by userspace (it sure is
> meant to be included and *is* included by libfuse and other stuff)
> THEN using  instead of  is fine regardless of
> whether __linux__ is defined or not.

That's probably true. But the patch in question adds the __KERNEL__
test, and *that* seems required.

If you think that we should instead drop the __linux__ test, than yes,
that part sounds fine. I thought that by "linux internal header" you
meant the fuse.h file, but you seem to mean the indirectly included
. That's fine.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 5:59 PM, Linus Torvalds
 wrote:
> On Tue, Apr 16, 2013 at 7:21 AM, Miklos Szeredi  wrote:
>>
>> And I still disagree.  Why should userspace use the linux internal
>> header when there's a perfectly good standard header that it can use?
>
> If it's called UAPI, it damn well is *meant* for user-space inclusion.
> Look at the file-name.
>
> And since the bug comment says "This file defines the kernel interface
> of FUSE" *AND* it very clearly has explicit code to support user-space
> includes with special user-space-only type defines, then your email is
> obviously just pure crap, and I don't understand how you can write
> that sentence with a straight face.

I think I meant something different by that sentence than what you
think I meant :)

What I meant is IF  is included by userspace (it sure is
meant to be included and *is* included by libfuse and other stuff)
THEN using  instead of  is fine regardless of
whether __linux__ is defined or not.

Does that sound better, or is there still something we disagree about?

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Linus Torvalds
On Tue, Apr 16, 2013 at 7:21 AM, Miklos Szeredi  wrote:
>
> And I still disagree.  Why should userspace use the linux internal
> header when there's a perfectly good standard header that it can use?

If it's called UAPI, it damn well is *meant* for user-space inclusion.
Look at the file-name.

And since the bug comment says "This file defines the kernel interface
of FUSE" *AND* it very clearly has explicit code to support user-space
includes with special user-space-only type defines, then your email is
obviously just pure crap, and I don't understand how you can write
that sentence with a straight face.

The *whole* point of the UAPI includes is two-fold:

 - to make it easier for user-space libraries to get at the kernel
definitions. Not everybody wants to use glibc for various reasons, and
where do you want people to *get* these declarations from?

 - to make kernel people more AWARE of when they are changing stuff
that affects user-space.

Now, the uapi model not perfect, but there are damn good reasons to at
least *strive* for both of those things, so I really don't understand
your comment there.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Mon, Apr 15, 2013 at 11:47 PM, Colin Cross  wrote:
> On Mon, Apr 15, 2013 at 1:41 PM, Colin Cross  wrote:
>> Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
>> header with one used in library) added #ifdef __linux__ around
>> defines if it is not set.  The kernel build is self-contained and
>> can be built on non-Linux toolchains.  After the mentioned commit
>> builds on non-Linux toolchains will try to include stdint.h and
>> fail due to -nostdinc, and then fail with a bunch of undefined
>> type errors.
>>
>> Change the #ifdef to check for __linux__ or __KERNEL__ so that
>> it uses the kernel typedefs if __KERNEL__ is set.
>>
>> Signed-off-by: Colin Cross 
>> ---
>> I think this should go in v3.9, without it bare-metal toolchains
>> that don't define __linux__ will fail to compile the kernel, and
>> cross-compiles from non-linux hosts will probably also fail.
>
> Miklos, I see Arve sent an equivalent patch to you a month ago
> (https://lkml.org/lkml/2013/3/11/620), and I agree with his response
> to your question (checking for __linux__ and __KERNEL__ is better than
> just checking for __KERNEL__ if you want this header to be used in
> userspace builds on linux targets).

And I still disagree.  Why should userspace use the linux internal
header when there's a perfectly good standard header that it can use?

Patch committed and pushed to for-next and for-linus.  I'll push to
Linus tomorrow.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Mon, Apr 15, 2013 at 11:47 PM, Colin Cross ccr...@android.com wrote:
 On Mon, Apr 15, 2013 at 1:41 PM, Colin Cross ccr...@android.com wrote:
 Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
 header with one used in library) added #ifdef __linux__ around
 defines if it is not set.  The kernel build is self-contained and
 can be built on non-Linux toolchains.  After the mentioned commit
 builds on non-Linux toolchains will try to include stdint.h and
 fail due to -nostdinc, and then fail with a bunch of undefined
 type errors.

 Change the #ifdef to check for __linux__ or __KERNEL__ so that
 it uses the kernel typedefs if __KERNEL__ is set.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
 I think this should go in v3.9, without it bare-metal toolchains
 that don't define __linux__ will fail to compile the kernel, and
 cross-compiles from non-linux hosts will probably also fail.

 Miklos, I see Arve sent an equivalent patch to you a month ago
 (https://lkml.org/lkml/2013/3/11/620), and I agree with his response
 to your question (checking for __linux__ and __KERNEL__ is better than
 just checking for __KERNEL__ if you want this header to be used in
 userspace builds on linux targets).

And I still disagree.  Why should userspace use the linux internal
header when there's a perfectly good standard header that it can use?

Patch committed and pushed to for-next and for-linus.  I'll push to
Linus tomorrow.

Thanks,
Miklos
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Linus Torvalds
On Tue, Apr 16, 2013 at 7:21 AM, Miklos Szeredi mik...@szeredi.hu wrote:

 And I still disagree.  Why should userspace use the linux internal
 header when there's a perfectly good standard header that it can use?

If it's called UAPI, it damn well is *meant* for user-space inclusion.
Look at the file-name.

And since the bug comment says This file defines the kernel interface
of FUSE *AND* it very clearly has explicit code to support user-space
includes with special user-space-only type defines, then your email is
obviously just pure crap, and I don't understand how you can write
that sentence with a straight face.

The *whole* point of the UAPI includes is two-fold:

 - to make it easier for user-space libraries to get at the kernel
definitions. Not everybody wants to use glibc for various reasons, and
where do you want people to *get* these declarations from?

 - to make kernel people more AWARE of when they are changing stuff
that affects user-space.

Now, the uapi model not perfect, but there are damn good reasons to at
least *strive* for both of those things, so I really don't understand
your comment there.

Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 5:59 PM, Linus Torvalds
torva...@linux-foundation.org wrote:
 On Tue, Apr 16, 2013 at 7:21 AM, Miklos Szeredi mik...@szeredi.hu wrote:

 And I still disagree.  Why should userspace use the linux internal
 header when there's a perfectly good standard header that it can use?

 If it's called UAPI, it damn well is *meant* for user-space inclusion.
 Look at the file-name.

 And since the bug comment says This file defines the kernel interface
 of FUSE *AND* it very clearly has explicit code to support user-space
 includes with special user-space-only type defines, then your email is
 obviously just pure crap, and I don't understand how you can write
 that sentence with a straight face.

I think I meant something different by that sentence than what you
think I meant :)

What I meant is IF linux/fuse.h is included by userspace (it sure is
meant to be included and *is* included by libfuse and other stuff)
THEN using stdint.h instead of linux/types.h is fine regardless of
whether __linux__ is defined or not.

Does that sound better, or is there still something we disagree about?

Thanks,
Miklos
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Linus Torvalds
 What I meant is IF linux/fuse.h is included by userspace (it sure is
 meant to be included and *is* included by libfuse and other stuff)
 THEN using stdint.h instead of linux/types.h is fine regardless of
 whether __linux__ is defined or not.

That's probably true. But the patch in question adds the __KERNEL__
test, and *that* seems required.

If you think that we should instead drop the __linux__ test, than yes,
that part sounds fine. I thought that by linux internal header you
meant the fuse.h file, but you seem to mean the indirectly included
inux/types.h. That's fine.

Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Colin Cross
On Tue, Apr 16, 2013 at 11:21 AM, Linus Torvalds
torva...@linux-foundation.org wrote:
 What I meant is IF linux/fuse.h is included by userspace (it sure is
 meant to be included and *is* included by libfuse and other stuff)
 THEN using stdint.h instead of linux/types.h is fine regardless of
 whether __linux__ is defined or not.

 That's probably true. But the patch in question adds the __KERNEL__
 test, and *that* seems required.

 If you think that we should instead drop the __linux__ test, than yes,
 that part sounds fine. I thought that by linux internal header you
 meant the fuse.h file, but you seem to mean the indirectly included
 inux/types.h. That's fine.

 Linus

Dropping __linux__ causes a make headers_check warning, which the
kbuild test robot reported this morning:
usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
#include linux/types.h
Using my patch without modification does not cause that warning.

linux/types.h defines the types that are used to communicate between
the kernel and userspace.  Redefining those in each header makes no
sense, and will probably cause redefined types warnings if you compile
a userspace file that includes fuse.h and another uapi header that
properly includes linux/types.h.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Miklos Szeredi
On Tue, Apr 16, 2013 at 8:29 PM, Colin Cross ccr...@android.com wrote:
 Dropping __linux__ causes a make headers_check warning, which the
 kbuild test robot reported this morning:
 usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
 #include linux/types.h
 Using my patch without modification does not cause that warning.

 linux/types.h defines the types that are used to communicate between
 the kernel and userspace.  Redefining those in each header makes no
 sense, and will probably cause redefined types warnings if you compile
 a userspace file that includes fuse.h and another uapi header that
 properly includes linux/types.h.

linux/types.h is linux specific while the fuse API is cross
platform.  So the userspace header obviously has to use some cross
platform type definitions.  Making the type definitions depend on
__linux__ is probably just as unreliable in userspace as it is in the
kernel, so why complicate things with that?

Thanks,
Miklos
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-16 Thread Colin Cross
On Tue, Apr 16, 2013 at 12:11 PM, Miklos Szeredi mik...@szeredi.hu wrote:
 On Tue, Apr 16, 2013 at 8:29 PM, Colin Cross ccr...@android.com wrote:
 Dropping __linux__ causes a make headers_check warning, which the
 kbuild test robot reported this morning:
 usr/include/linux/fuse.h:99: found __[us]{8,16,32,64} type without
 #include linux/types.h
 Using my patch without modification does not cause that warning.

 linux/types.h defines the types that are used to communicate between
 the kernel and userspace.  Redefining those in each header makes no
 sense, and will probably cause redefined types warnings if you compile
 a userspace file that includes fuse.h and another uapi header that
 properly includes linux/types.h.

 linux/types.h is linux specific while the fuse API is cross
 platform.  So the userspace header obviously has to use some cross
 platform type definitions.  Making the type definitions depend on
 __linux__ is probably just as unreliable in userspace as it is in the
 kernel, so why complicate things with that?

I'm not sure what you mean by __linux__ being unreliable, it is set by
any toolchain that is compield with linux as its target.  It should be
very reliable in userspace, its only unreliable in the kernel because
the kernel doesn't rely on any specific os target in the toolchain, it
is self contained.

With your change, compiling a program that includes linux/fuse.h and
linux/icmp.h against the headers installed by make headers_installed
results in some strange preprocessor results.  The lines in
int-ll64.h, which is what normally defines the userspace types, are:
typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short __u16;

typedef __signed__ int __s32;
typedef unsigned int __u32;

After preprocessing with your fuse.h included before icmp.h (which
includes linux/types.h, which eventually includes
asm-generic/int-ll64.h):
typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short uint16_t;

typedef __signed__ int int32_t;
typedef unsigned int uint32_t;

The first three lines are the kernel headers providing typedefs for
the types that are not used in fuse.h, and therefore not covered by
the #define hacks there.  The last three lines are a problem: now you
have redefined the stdint types to match the kernel's types, better
hope they match the host's stdint definitions or you'll get redefined
type warnings.

Every other uapi header includes linux/types.h to get its type
definitions, and fuse.h should do the same when compiling for
userspace targeting linux or when compiling the kernel.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-15 Thread Colin Cross
On Mon, Apr 15, 2013 at 1:41 PM, Colin Cross  wrote:
> Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
> header with one used in library) added #ifdef __linux__ around
> defines if it is not set.  The kernel build is self-contained and
> can be built on non-Linux toolchains.  After the mentioned commit
> builds on non-Linux toolchains will try to include stdint.h and
> fail due to -nostdinc, and then fail with a bunch of undefined
> type errors.
>
> Change the #ifdef to check for __linux__ or __KERNEL__ so that
> it uses the kernel typedefs if __KERNEL__ is set.
>
> Signed-off-by: Colin Cross 
> ---
> I think this should go in v3.9, without it bare-metal toolchains
> that don't define __linux__ will fail to compile the kernel, and
> cross-compiles from non-linux hosts will probably also fail.

Miklos, I see Arve sent an equivalent patch to you a month ago
(https://lkml.org/lkml/2013/3/11/620), and I agree with his response
to your question (checking for __linux__ and __KERNEL__ is better than
just checking for __KERNEL__ if you want this header to be used in
userspace builds on linux targets).  Can you pull one of these two
patches before 3.9?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-15 Thread Colin Cross
Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
header with one used in library) added #ifdef __linux__ around
defines if it is not set.  The kernel build is self-contained and
can be built on non-Linux toolchains.  After the mentioned commit
builds on non-Linux toolchains will try to include stdint.h and
fail due to -nostdinc, and then fail with a bunch of undefined
type errors.

Change the #ifdef to check for __linux__ or __KERNEL__ so that
it uses the kernel typedefs if __KERNEL__ is set.

Signed-off-by: Colin Cross 
---
I think this should go in v3.9, without it bare-metal toolchains
that don't define __linux__ will fail to compile the kernel, and
cross-compiles from non-linux hosts will probably also fail.

 include/uapi/linux/fuse.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 4c43b44..5b79443 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -95,7 +95,7 @@
 #ifndef _LINUX_FUSE_H
 #define _LINUX_FUSE_H
 
-#ifdef __linux__
+#if defined(__linux__) || defined(__KERNEL__)
 #include 
 #else
 #include 
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-15 Thread Colin Cross
Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
header with one used in library) added #ifdef __linux__ around
defines if it is not set.  The kernel build is self-contained and
can be built on non-Linux toolchains.  After the mentioned commit
builds on non-Linux toolchains will try to include stdint.h and
fail due to -nostdinc, and then fail with a bunch of undefined
type errors.

Change the #ifdef to check for __linux__ or __KERNEL__ so that
it uses the kernel typedefs if __KERNEL__ is set.

Signed-off-by: Colin Cross ccr...@android.com
---
I think this should go in v3.9, without it bare-metal toolchains
that don't define __linux__ will fail to compile the kernel, and
cross-compiles from non-linux hosts will probably also fail.

 include/uapi/linux/fuse.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 4c43b44..5b79443 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -95,7 +95,7 @@
 #ifndef _LINUX_FUSE_H
 #define _LINUX_FUSE_H
 
-#ifdef __linux__
+#if defined(__linux__) || defined(__KERNEL__)
 #include linux/types.h
 #else
 #include stdint.h
-- 
1.8.1.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fuse: use kernel headers when __KERNEL__ is set

2013-04-15 Thread Colin Cross
On Mon, Apr 15, 2013 at 1:41 PM, Colin Cross ccr...@android.com wrote:
 Commit 7e98d53086d18c877cb44e9065219335184024de (Synchronize fuse
 header with one used in library) added #ifdef __linux__ around
 defines if it is not set.  The kernel build is self-contained and
 can be built on non-Linux toolchains.  After the mentioned commit
 builds on non-Linux toolchains will try to include stdint.h and
 fail due to -nostdinc, and then fail with a bunch of undefined
 type errors.

 Change the #ifdef to check for __linux__ or __KERNEL__ so that
 it uses the kernel typedefs if __KERNEL__ is set.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
 I think this should go in v3.9, without it bare-metal toolchains
 that don't define __linux__ will fail to compile the kernel, and
 cross-compiles from non-linux hosts will probably also fail.

Miklos, I see Arve sent an equivalent patch to you a month ago
(https://lkml.org/lkml/2013/3/11/620), and I agree with his response
to your question (checking for __linux__ and __KERNEL__ is better than
just checking for __KERNEL__ if you want this header to be used in
userspace builds on linux targets).  Can you pull one of these two
patches before 3.9?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/