| Message ID | b8019eac434b2e61813a2061090d85401c6c44c4.1527310210.git.keno@alumni.harvard.edu |
|---|---|
| State | New |
| Headers | show |
| Series | 9p: Add support for Darwin | expand |
Hi Keno, On 05/26/2018 02:23 AM, keno@juliacomputing.com wrote: > From: Keno Fischer <keno@alumni.harvard.edu> > > - Guard two Linux only headers. > - Define `ENOATTR` only if not only defined > (it's defined in system headers on Darwin). > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > --- > fsdev/file-op-9p.h | 2 ++ > hw/9pfs/9p-local.c | 2 ++ > include/qemu/xattr.h | 4 +++- > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h > index 3fa062b..a13e729 100644 > --- a/fsdev/file-op-9p.h > +++ b/fsdev/file-op-9p.h > @@ -16,7 +16,9 @@ > > #include <dirent.h> > #include <utime.h> > +#ifdef CONFIG_LINUX What about a less restrictive: #ifndef __APPLE__ > #include <sys/vfs.h> > +#endif > #include "qemu-fsdev-throttle.h" > > #define SM_LOCAL_MODE_BITS 0600 > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index b37b1db..f6c7526 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -27,10 +27,12 @@ > #include "qemu/error-report.h" > #include "qemu/option.h" > #include <libgen.h> > +#ifdef CONFIG_LINUX > #include <linux/fs.h> > #ifdef CONFIG_LINUX_MAGIC_H > #include <linux/magic.h> > #endif > +#endif > #include <sys/ioctl.h> > > #ifndef XFS_SUPER_MAGIC > diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h > index a83fe8e..f1d0f7b 100644 > --- a/include/qemu/xattr.h > +++ b/include/qemu/xattr.h > @@ -22,7 +22,9 @@ > #ifdef CONFIG_LIBATTR > # include <attr/xattr.h> > #else > -# define ENOATTR ENODATA > +# if !defined(ENOATTR) > +# define ENOATTR ENODATA > +# endif > # include <sys/xattr.h> > #endif > Rest looks correct.
On 26 May 2018 at 07:30, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Keno, > > On 05/26/2018 02:23 AM, keno@juliacomputing.com wrote: >> From: Keno Fischer <keno@alumni.harvard.edu> >> >> - Guard two Linux only headers. >> - Define `ENOATTR` only if not only defined >> (it's defined in system headers on Darwin). >> >> Signed-off-by: Keno Fischer <keno@juliacomputing.com> >> --- >> fsdev/file-op-9p.h | 2 ++ >> hw/9pfs/9p-local.c | 2 ++ >> include/qemu/xattr.h | 4 +++- >> 3 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h >> index 3fa062b..a13e729 100644 >> --- a/fsdev/file-op-9p.h >> +++ b/fsdev/file-op-9p.h >> @@ -16,7 +16,9 @@ >> >> #include <dirent.h> >> #include <utime.h> >> +#ifdef CONFIG_LINUX > > What about a less restrictive: > > #ifndef __APPLE__ In general I would recommend checking for specific features (usually in configure), rather than adding ifdef tests for particular OSes. In this case presumably we're including these headers because we're after a specific function or define or whatever, so we can check in configure for what header that's in (or if it's not available at all). thanks -- PMM
>>> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h >>> index 3fa062b..a13e729 100644 >>> --- a/fsdev/file-op-9p.h >>> +++ b/fsdev/file-op-9p.h >>> @@ -16,7 +16,9 @@ >>> >>> #include <dirent.h> >>> #include <utime.h> >>> +#ifdef CONFIG_LINUX >> >> What about a less restrictive: >> >> #ifndef __APPLE__ > > In general I would recommend checking for specific > features (usually in configure), rather than adding > ifdef tests for particular OSes. In this case presumably > we're including these headers because we're after > a specific function or define or whatever, so we can > check in configure for what header that's in (or > if it's not available at all). > > thanks > -- PMM This header is used for struct statfs. I would be fine with a configure check for this, though it looks like other places in the code base that use this header (e.g. util/mmap-alloc.c) also guard it in CONFIG_LINUX, so that seemed like the right check to me given the current code base. Would you like me to submit a patch to switch these to a configure check?
On Sat, 26 May 2018 01:23:03 -0400 keno@juliacomputing.com wrote: > From: Keno Fischer <keno@alumni.harvard.edu> > > - Guard two Linux only headers. > - Define `ENOATTR` only if not only defined > (it's defined in system headers on Darwin). > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > --- > fsdev/file-op-9p.h | 2 ++ > hw/9pfs/9p-local.c | 2 ++ > include/qemu/xattr.h | 4 +++- Irrespectively of the discussion on checking this in configure, there's another user of <sys/vfs.h> in fsdev/virtfs-proxy-helper.c. I see in patch 13 that the helper won't be built on Darwin, but this could change, and anyway, I'd like the handling of <sys/vfs.h> to stay consistent across the VirtFS code. > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h > index 3fa062b..a13e729 100644 > --- a/fsdev/file-op-9p.h > +++ b/fsdev/file-op-9p.h > @@ -16,7 +16,9 @@ > > #include <dirent.h> > #include <utime.h> > +#ifdef CONFIG_LINUX > #include <sys/vfs.h> > +#endif > #include "qemu-fsdev-throttle.h" > > #define SM_LOCAL_MODE_BITS 0600 > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index b37b1db..f6c7526 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -27,10 +27,12 @@ > #include "qemu/error-report.h" > #include "qemu/option.h" > #include <libgen.h> > +#ifdef CONFIG_LINUX > #include <linux/fs.h> > #ifdef CONFIG_LINUX_MAGIC_H > #include <linux/magic.h> > #endif > +#endif > #include <sys/ioctl.h> > > #ifndef XFS_SUPER_MAGIC > diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h > index a83fe8e..f1d0f7b 100644 > --- a/include/qemu/xattr.h > +++ b/include/qemu/xattr.h > @@ -22,7 +22,9 @@ > #ifdef CONFIG_LIBATTR > # include <attr/xattr.h> > #else > -# define ENOATTR ENODATA > +# if !defined(ENOATTR) > +# define ENOATTR ENODATA > +# endif > # include <sys/xattr.h> > #endif >
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index 3fa062b..a13e729 100644 --- a/fsdev/file-op-9p.h +++ b/fsdev/file-op-9p.h @@ -16,7 +16,9 @@ #include <dirent.h> #include <utime.h> +#ifdef CONFIG_LINUX #include <sys/vfs.h> +#endif #include "qemu-fsdev-throttle.h" #define SM_LOCAL_MODE_BITS 0600 diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index b37b1db..f6c7526 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -27,10 +27,12 @@ #include "qemu/error-report.h" #include "qemu/option.h" #include <libgen.h> +#ifdef CONFIG_LINUX #include <linux/fs.h> #ifdef CONFIG_LINUX_MAGIC_H #include <linux/magic.h> #endif +#endif #include <sys/ioctl.h> #ifndef XFS_SUPER_MAGIC diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h index a83fe8e..f1d0f7b 100644 --- a/include/qemu/xattr.h +++ b/include/qemu/xattr.h @@ -22,7 +22,9 @@ #ifdef CONFIG_LIBATTR # include <attr/xattr.h> #else -# define ENOATTR ENODATA +# if !defined(ENOATTR) +# define ENOATTR ENODATA +# endif # include <sys/xattr.h> #endif