[06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

This will help reduce the qemu-common.h dependency hell.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
--
Changes v1 -> v2:
 - move qemu_open() & qemu_close() to qemu-stdio.h, too
---
 qemu-common.h | 59 ++--------------------------------------------
 qemu-stdio.h  | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 57 deletions(-)
 create mode 100644 qemu-stdio.h

On Fri,  9 Nov 2012 12:56:34 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This will help reduce the qemu-common.h dependency hell.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> --
> Changes v1 -> v2:
>  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> ---
>  qemu-common.h | 59 ++--------------------------------------------
>  qemu-stdio.h  | 76
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> changed, 78 insertions(+), 57 deletions(-) create mode 100644 qemu-stdio.h
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index 6441bee..5080382 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -15,6 +15,8 @@
>  #include "compiler.h"
>  #include "config-host.h"
>  
> +#include "qemu-stdio.h"
> +
>  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
>  #endif
> @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
>  #include "qemu-os-posix.h"
>  #endif
>  
> -#ifndef O_LARGEFILE
> -#define O_LARGEFILE 0
> -#endif
> -#ifndef O_BINARY
> -#define O_BINARY 0
> -#endif
> -#ifndef MAP_ANONYMOUS
> -#define MAP_ANONYMOUS MAP_ANON
> -#endif
> -#ifndef ENOMEDIUM
> -#define ENOMEDIUM ENODEV
> -#endif
> -#if !defined(ENOTSUP)
> -#define ENOTSUP 4096
> -#endif
> -#if !defined(ECANCELED)
> -#define ECANCELED 4097
> -#endif
> -#ifndef TIME_MAX
> -#define TIME_MAX LONG_MAX
> -#endif
> -
>  /* HOST_LONG_BITS is the size of a native pointer in bits. */
>  #if UINTPTR_MAX == UINT32_MAX
>  # define HOST_LONG_BITS 32
> @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
>  # error Unknown pointer size
>  #endif
>  
> -#ifndef CONFIG_IOVEC
> -#define CONFIG_IOVEC
> -struct iovec {
> -    void *iov_base;
> -    size_t iov_len;
> -};
> -/*
> - * Use the same value as Linux for now.
> - */
> -#define IOV_MAX		1024
> -#else
> -#include <sys/uio.h>
> -#endif
> -
> -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> -    GCC_FMT_ATTR(2, 3);
Is there any particular reason to move fprintf_function from qemu-common.h?

> -
> -#ifdef _WIN32
> -#define fsync _commit
> -#if !defined(lseek)
> -# define lseek _lseeki64
> -#endif
> -int qemu_ftruncate64(int, int64_t);
> -#if !defined(ftruncate)
> -# define ftruncate qemu_ftruncate64
> -#endif
> -
> -static inline char *realpath(const char *path, char *resolved_path)
> -{
> -    _fullpath(resolved_path, path, _MAX_PATH);
> -    return resolved_path;
> -}
> -#endif
>  
>  /* icount */
>  void configure_icount(const char *option);
> @@ -217,8 +164,6 @@ const char *path(const char *pathname);
>  
>  void *qemu_oom_check(void *ptr);
>  
> -int qemu_open(const char *name, int flags, ...);
> -int qemu_close(int fd);
>  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>      QEMU_WARN_UNUSED_RESULT;
>  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
2 above funcs could be moved along with qemu_open() to osdep.h

> diff --git a/qemu-stdio.h b/qemu-stdio.h
> new file mode 100644
> index 0000000..b2e8eda
> --- /dev/null
> +++ b/qemu-stdio.h
> @@ -0,0 +1,76 @@
> +/* Some basic definitions related to stdio.h or other I/O interfaces
> + */
> +#ifndef QEMU_STDIO_H
> +#define QEMU_STDIO_H
> +
> +#include "compiler.h"
> +#include "config-host.h"
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <sys/mman.h>
> +
> +#ifndef O_LARGEFILE
> +#define O_LARGEFILE 0
> +#endif
> +#ifndef O_BINARY
> +#define O_BINARY 0
> +#endif
> +#ifndef MAP_ANONYMOUS
> +#define MAP_ANONYMOUS MAP_ANON
> +#endif
> +#ifndef ENOMEDIUM
> +#define ENOMEDIUM ENODEV
> +#endif
> +#if !defined(ENOTSUP)
> +#define ENOTSUP 4096
> +#endif
> +#if !defined(ECANCELED)
> +#define ECANCELED 4097
> +#endif
> +#ifndef TIME_MAX
> +#define TIME_MAX LONG_MAX
> +#endif
> +
> +#ifndef CONFIG_IOVEC
> +#define CONFIG_IOVEC
> +struct iovec {
> +    void *iov_base;
> +    size_t iov_len;
> +};
> +/*
> + * Use the same value as Linux for now.
> + */
> +#define IOV_MAX     1024
> +#else
> +#include <sys/uio.h>
> +#endif
> +
> +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> +    GCC_FMT_ATTR(2, 3);
> +
> +#ifdef _WIN32
> +#define fsync _commit
> +#if !defined(lseek)
> +# define lseek _lseeki64
> +#endif
> +int qemu_ftruncate64(int, int64_t);
> +#if !defined(ftruncate)
> +# define ftruncate qemu_ftruncate64
> +#endif
> +
> +static inline char *realpath(const char *path, char *resolved_path)
> +{
> +    _fullpath(resolved_path, path, _MAX_PATH);
> +    return resolved_path;
> +}
> +#endif
> +
> +int qemu_open(const char *name, int flags, ...);
> +int qemu_close(int fd);
qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
better to move their declaration to osdep.h.
The rest looks like it fits a purpose of osdep.h as well, so probably it
could be moved there and we could avoid creating a new header.

> +
> +#endif /* QEMU_STDIO_H */

On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> On Fri,  9 Nov 2012 12:56:34 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > This will help reduce the qemu-common.h dependency hell.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > --
> > Changes v1 -> v2:
> >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > ---
> >  qemu-common.h | 59 ++--------------------------------------------
> >  qemu-stdio.h  | 76
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > changed, 78 insertions(+), 57 deletions(-) create mode 100644 qemu-stdio.h
> > 
> > diff --git a/qemu-common.h b/qemu-common.h
> > index 6441bee..5080382 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -15,6 +15,8 @@
> >  #include "compiler.h"
> >  #include "config-host.h"
> >  
> > +#include "qemu-stdio.h"
> > +
> >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> >  #endif
> > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> >  #include "qemu-os-posix.h"
> >  #endif
> >  
> > -#ifndef O_LARGEFILE
> > -#define O_LARGEFILE 0
> > -#endif
> > -#ifndef O_BINARY
> > -#define O_BINARY 0
> > -#endif
> > -#ifndef MAP_ANONYMOUS
> > -#define MAP_ANONYMOUS MAP_ANON
> > -#endif
> > -#ifndef ENOMEDIUM
> > -#define ENOMEDIUM ENODEV
> > -#endif
> > -#if !defined(ENOTSUP)
> > -#define ENOTSUP 4096
> > -#endif
> > -#if !defined(ECANCELED)
> > -#define ECANCELED 4097
> > -#endif
> > -#ifndef TIME_MAX
> > -#define TIME_MAX LONG_MAX
> > -#endif
> > -
> >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> >  #if UINTPTR_MAX == UINT32_MAX
> >  # define HOST_LONG_BITS 32
> > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> >  # error Unknown pointer size
> >  #endif
> >  
> > -#ifndef CONFIG_IOVEC
> > -#define CONFIG_IOVEC
> > -struct iovec {
> > -    void *iov_base;
> > -    size_t iov_len;
> > -};
> > -/*
> > - * Use the same value as Linux for now.
> > - */
> > -#define IOV_MAX		1024
> > -#else
> > -#include <sys/uio.h>
> > -#endif
> > -
> > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > -    GCC_FMT_ATTR(2, 3);
> Is there any particular reason to move fprintf_function from qemu-common.h?

Yes: the reason is that qemu-common.h should die.  ;-)

Long explanation:
1) qemu-common.h has too much stuff inside it;
2) Due to (1), having headers that include qemu-common.h easily lead to
   circular header dependencies;
3) Due to (2), header files should not include qemu-common.h;
4) Headers that use fprintf_function can't make sure fprintf_function is
   defined, because of (3).


> 
> > -
> > -#ifdef _WIN32
> > -#define fsync _commit
> > -#if !defined(lseek)
> > -# define lseek _lseeki64
> > -#endif
> > -int qemu_ftruncate64(int, int64_t);
> > -#if !defined(ftruncate)
> > -# define ftruncate qemu_ftruncate64
> > -#endif
> > -
> > -static inline char *realpath(const char *path, char *resolved_path)
> > -{
> > -    _fullpath(resolved_path, path, _MAX_PATH);
> > -    return resolved_path;
> > -}
> > -#endif
> >  
> >  /* icount */
> >  void configure_icount(const char *option);
> > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> >  
> >  void *qemu_oom_check(void *ptr);
> >  
> > -int qemu_open(const char *name, int flags, ...);
> > -int qemu_close(int fd);
> >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> >      QEMU_WARN_UNUSED_RESULT;
> >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
> 2 above funcs could be moved along with qemu_open() to osdep.h

I would happily move them, if somebody gave me an one-line description
of what's the purpose of osdep.h (so I know what is supposed to be
there, and what's not supposed to be there).

> 
> > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > new file mode 100644
> > index 0000000..b2e8eda
> > --- /dev/null
> > +++ b/qemu-stdio.h
> > @@ -0,0 +1,76 @@
> > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > + */
> > +#ifndef QEMU_STDIO_H
> > +#define QEMU_STDIO_H
> > +
> > +#include "compiler.h"
> > +#include "config-host.h"
> > +
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <fcntl.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <sys/time.h>
> > +#include <sys/mman.h>
> > +
> > +#ifndef O_LARGEFILE
> > +#define O_LARGEFILE 0
> > +#endif
> > +#ifndef O_BINARY
> > +#define O_BINARY 0
> > +#endif
> > +#ifndef MAP_ANONYMOUS
> > +#define MAP_ANONYMOUS MAP_ANON
> > +#endif
> > +#ifndef ENOMEDIUM
> > +#define ENOMEDIUM ENODEV
> > +#endif
> > +#if !defined(ENOTSUP)
> > +#define ENOTSUP 4096
> > +#endif
> > +#if !defined(ECANCELED)
> > +#define ECANCELED 4097
> > +#endif
> > +#ifndef TIME_MAX
> > +#define TIME_MAX LONG_MAX
> > +#endif
> > +
> > +#ifndef CONFIG_IOVEC
> > +#define CONFIG_IOVEC
> > +struct iovec {
> > +    void *iov_base;
> > +    size_t iov_len;
> > +};
> > +/*
> > + * Use the same value as Linux for now.
> > + */
> > +#define IOV_MAX     1024
> > +#else
> > +#include <sys/uio.h>
> > +#endif
> > +
> > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > +    GCC_FMT_ATTR(2, 3);
> > +
> > +#ifdef _WIN32
> > +#define fsync _commit
> > +#if !defined(lseek)
> > +# define lseek _lseeki64
> > +#endif
> > +int qemu_ftruncate64(int, int64_t);
> > +#if !defined(ftruncate)
> > +# define ftruncate qemu_ftruncate64
> > +#endif
> > +
> > +static inline char *realpath(const char *path, char *resolved_path)
> > +{
> > +    _fullpath(resolved_path, path, _MAX_PATH);
> > +    return resolved_path;
> > +}
> > +#endif
> > +
> > +int qemu_open(const char *name, int flags, ...);
> > +int qemu_close(int fd);
> qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> better to move their declaration to osdep.h.
> The rest looks like it fits a purpose of osdep.h as well, so probably it
> could be moved there and we could avoid creating a new header.

Maybe, but I really would like to have a good description of what's the
meaning/purpose of osdep.h, first. osdep.h seems to have lots of
unrelated stuff inside it, already, and I don't want to risk making
osdep.h become a new catchall header file, like qemu-common.h is today.

> 
> > +
> > +#endif /* QEMU_STDIO_H */
>

On Tue, 13 Nov 2012 13:52:16 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> > On Fri,  9 Nov 2012 12:56:34 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > This will help reduce the qemu-common.h dependency hell.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > --
> > > Changes v1 -> v2:
> > >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > > ---
> > >  qemu-common.h | 59 ++--------------------------------------------
> > >  qemu-stdio.h  | 76
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > > changed, 78 insertions(+), 57 deletions(-) create mode 100644
> > > qemu-stdio.h
> > > 
> > > diff --git a/qemu-common.h b/qemu-common.h
> > > index 6441bee..5080382 100644
> > > --- a/qemu-common.h
> > > +++ b/qemu-common.h
> > > @@ -15,6 +15,8 @@
> > >  #include "compiler.h"
> > >  #include "config-host.h"
> > >  
> > > +#include "qemu-stdio.h"
> > > +
> > >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> > >  #endif
> > > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> > >  #include "qemu-os-posix.h"
> > >  #endif
> > >  
> > > -#ifndef O_LARGEFILE
> > > -#define O_LARGEFILE 0
> > > -#endif
> > > -#ifndef O_BINARY
> > > -#define O_BINARY 0
> > > -#endif
> > > -#ifndef MAP_ANONYMOUS
> > > -#define MAP_ANONYMOUS MAP_ANON
> > > -#endif
> > > -#ifndef ENOMEDIUM
> > > -#define ENOMEDIUM ENODEV
> > > -#endif
> > > -#if !defined(ENOTSUP)
> > > -#define ENOTSUP 4096
> > > -#endif
> > > -#if !defined(ECANCELED)
> > > -#define ECANCELED 4097
> > > -#endif
> > > -#ifndef TIME_MAX
> > > -#define TIME_MAX LONG_MAX
> > > -#endif
> > > -
> > >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> > >  #if UINTPTR_MAX == UINT32_MAX
> > >  # define HOST_LONG_BITS 32
> > > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> > >  # error Unknown pointer size
> > >  #endif
> > >  
> > > -#ifndef CONFIG_IOVEC
> > > -#define CONFIG_IOVEC
> > > -struct iovec {
> > > -    void *iov_base;
> > > -    size_t iov_len;
> > > -};
> > > -/*
> > > - * Use the same value as Linux for now.
> > > - */
> > > -#define IOV_MAX		1024
> > > -#else
> > > -#include <sys/uio.h>
> > > -#endif
> > > -
> > > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > -    GCC_FMT_ATTR(2, 3);
> > Is there any particular reason to move fprintf_function from
> > qemu-common.h?
> 
> Yes: the reason is that qemu-common.h should die.  ;-)
> 
> Long explanation:
> 1) qemu-common.h has too much stuff inside it;
> 2) Due to (1), having headers that include qemu-common.h easily lead to
>    circular header dependencies;
> 3) Due to (2), header files should not include qemu-common.h;
> 4) Headers that use fprintf_function can't make sure fprintf_function is
>    defined, because of (3).
Strictly speaking this movement is not related/required for this series to
work, it compiles without it.
But if there is no complaints it's fine by me.

> 
> > 
> > > -
> > > -#ifdef _WIN32
> > > -#define fsync _commit
> > > -#if !defined(lseek)
> > > -# define lseek _lseeki64
> > > -#endif
> > > -int qemu_ftruncate64(int, int64_t);
> > > -#if !defined(ftruncate)
> > > -# define ftruncate qemu_ftruncate64
> > > -#endif
> > > -
> > > -static inline char *realpath(const char *path, char *resolved_path)
> > > -{
> > > -    _fullpath(resolved_path, path, _MAX_PATH);
> > > -    return resolved_path;
> > > -}
> > > -#endif
> > >  
> > >  /* icount */
> > >  void configure_icount(const char *option);
> > > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> > >  
> > >  void *qemu_oom_check(void *ptr);
> > >  
> > > -int qemu_open(const char *name, int flags, ...);
> > > -int qemu_close(int fd);
> > >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> > >      QEMU_WARN_UNUSED_RESULT;
> > >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int
> > > flags)
> > 2 above funcs could be moved along with qemu_open() to osdep.h
> 
> I would happily move them, if somebody gave me an one-line description
> of what's the purpose of osdep.h (so I know what is supposed to be
> there, and what's not supposed to be there).
they are defined in osdep.c, it's logical to have declarations in osdep.h

> 
> > 
> > > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > > new file mode 100644
> > > index 0000000..b2e8eda
> > > --- /dev/null
> > > +++ b/qemu-stdio.h
> > > @@ -0,0 +1,76 @@
> > > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > > + */
> > > +#ifndef QEMU_STDIO_H
> > > +#define QEMU_STDIO_H
> > > +
> > > +#include "compiler.h"
> > > +#include "config-host.h"
> > > +
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +#include <fcntl.h>
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/time.h>
> > > +#include <sys/mman.h>
> > > +
> > > +#ifndef O_LARGEFILE
> > > +#define O_LARGEFILE 0
> > > +#endif
> > > +#ifndef O_BINARY
> > > +#define O_BINARY 0
> > > +#endif
> > > +#ifndef MAP_ANONYMOUS
> > > +#define MAP_ANONYMOUS MAP_ANON
> > > +#endif
> > > +#ifndef ENOMEDIUM
> > > +#define ENOMEDIUM ENODEV
> > > +#endif
> > > +#if !defined(ENOTSUP)
> > > +#define ENOTSUP 4096
> > > +#endif
> > > +#if !defined(ECANCELED)
> > > +#define ECANCELED 4097
> > > +#endif
> > > +#ifndef TIME_MAX
> > > +#define TIME_MAX LONG_MAX
> > > +#endif
> > > +
> > > +#ifndef CONFIG_IOVEC
> > > +#define CONFIG_IOVEC
> > > +struct iovec {
> > > +    void *iov_base;
> > > +    size_t iov_len;
> > > +};
> > > +/*
> > > + * Use the same value as Linux for now.
> > > + */
> > > +#define IOV_MAX     1024
> > > +#else
> > > +#include <sys/uio.h>
> > > +#endif
> > > +
> > > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > +    GCC_FMT_ATTR(2, 3);
> > > +
> > > +#ifdef _WIN32
> > > +#define fsync _commit
> > > +#if !defined(lseek)
> > > +# define lseek _lseeki64
> > > +#endif
> > > +int qemu_ftruncate64(int, int64_t);
> > > +#if !defined(ftruncate)
> > > +# define ftruncate qemu_ftruncate64
> > > +#endif
> > > +
> > > +static inline char *realpath(const char *path, char *resolved_path)
> > > +{
> > > +    _fullpath(resolved_path, path, _MAX_PATH);
> > > +    return resolved_path;
> > > +}
> > > +#endif
> > > +
> > > +int qemu_open(const char *name, int flags, ...);
> > > +int qemu_close(int fd);
> > qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> > better to move their declaration to osdep.h.
> > The rest looks like it fits a purpose of osdep.h as well, so probably it
> > could be moved there and we could avoid creating a new header.
> 
> Maybe, but I really would like to have a good description of what's the
> meaning/purpose of osdep.h, first. osdep.h seems to have lots of
Judging from commit logs its purpose is in abstracting/unifying OS depended
features to some common denominator. And the code you are moving is
exactly does this.

> unrelated stuff inside it, already, and I don't want to risk making
> osdep.h become a new catchall header file, like qemu-common.h is today.
> 
So far osdep.h is pretty simple in terms of header dependences and doesn't
include catch-all headers, moving this declarations and defines there
doesn't complicate it in this aspect.

> > 
> > > +
> > > +#endif /* QEMU_STDIO_H */
> > 
>

On Tue, Nov 13, 2012 at 05:43:42PM +0100, Igor Mammedov wrote:
> On Tue, 13 Nov 2012 13:52:16 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> > > On Fri,  9 Nov 2012 12:56:34 -0200
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > 
> > > > This will help reduce the qemu-common.h dependency hell.
> > > > 
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > --
> > > > Changes v1 -> v2:
> > > >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > > > ---
> > > >  qemu-common.h | 59 ++--------------------------------------------
> > > >  qemu-stdio.h  | 76
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > > > changed, 78 insertions(+), 57 deletions(-) create mode 100644
> > > > qemu-stdio.h
> > > > 
> > > > diff --git a/qemu-common.h b/qemu-common.h
> > > > index 6441bee..5080382 100644
> > > > --- a/qemu-common.h
> > > > +++ b/qemu-common.h
> > > > @@ -15,6 +15,8 @@
> > > >  #include "compiler.h"
> > > >  #include "config-host.h"
> > > >  
> > > > +#include "qemu-stdio.h"
> > > > +
> > > >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > > > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> > > >  #endif
> > > > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> > > >  #include "qemu-os-posix.h"
> > > >  #endif
> > > >  
> > > > -#ifndef O_LARGEFILE
> > > > -#define O_LARGEFILE 0
> > > > -#endif
> > > > -#ifndef O_BINARY
> > > > -#define O_BINARY 0
> > > > -#endif
> > > > -#ifndef MAP_ANONYMOUS
> > > > -#define MAP_ANONYMOUS MAP_ANON
> > > > -#endif
> > > > -#ifndef ENOMEDIUM
> > > > -#define ENOMEDIUM ENODEV
> > > > -#endif
> > > > -#if !defined(ENOTSUP)
> > > > -#define ENOTSUP 4096
> > > > -#endif
> > > > -#if !defined(ECANCELED)
> > > > -#define ECANCELED 4097
> > > > -#endif
> > > > -#ifndef TIME_MAX
> > > > -#define TIME_MAX LONG_MAX
> > > > -#endif
> > > > -
> > > >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> > > >  #if UINTPTR_MAX == UINT32_MAX
> > > >  # define HOST_LONG_BITS 32
> > > > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> > > >  # error Unknown pointer size
> > > >  #endif
> > > >  
> > > > -#ifndef CONFIG_IOVEC
> > > > -#define CONFIG_IOVEC
> > > > -struct iovec {
> > > > -    void *iov_base;
> > > > -    size_t iov_len;
> > > > -};
> > > > -/*
> > > > - * Use the same value as Linux for now.
> > > > - */
> > > > -#define IOV_MAX		1024
> > > > -#else
> > > > -#include <sys/uio.h>
> > > > -#endif
> > > > -
> > > > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > > -    GCC_FMT_ATTR(2, 3);
> > > Is there any particular reason to move fprintf_function from
> > > qemu-common.h?
> > 
> > Yes: the reason is that qemu-common.h should die.  ;-)
> > 
> > Long explanation:
> > 1) qemu-common.h has too much stuff inside it;
> > 2) Due to (1), having headers that include qemu-common.h easily lead to
> >    circular header dependencies;
> > 3) Due to (2), header files should not include qemu-common.h;
> > 4) Headers that use fprintf_function can't make sure fprintf_function is
> >    defined, because of (3).
> Strictly speaking this movement is not related/required for this series to
> work, it compiles without it.

I think it was necessary when we were adding a new '#include "cpu.h"'
line somewhere, on previous versions of the series, but it may be not
necessary anymore.


> But if there is no complaints it's fine by me.

If the code compiles fine without the patch, I'm OK with dropping it by
now.  Then I can send a new version later that moves that code to
osdep.h.

> 
> > 
> > > 
> > > > -
> > > > -#ifdef _WIN32
> > > > -#define fsync _commit
> > > > -#if !defined(lseek)
> > > > -# define lseek _lseeki64
> > > > -#endif
> > > > -int qemu_ftruncate64(int, int64_t);
> > > > -#if !defined(ftruncate)
> > > > -# define ftruncate qemu_ftruncate64
> > > > -#endif
> > > > -
> > > > -static inline char *realpath(const char *path, char *resolved_path)
> > > > -{
> > > > -    _fullpath(resolved_path, path, _MAX_PATH);
> > > > -    return resolved_path;
> > > > -}
> > > > -#endif
> > > >  
> > > >  /* icount */
> > > >  void configure_icount(const char *option);
> > > > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> > > >  
> > > >  void *qemu_oom_check(void *ptr);
> > > >  
> > > > -int qemu_open(const char *name, int flags, ...);
> > > > -int qemu_close(int fd);
> > > >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> > > >      QEMU_WARN_UNUSED_RESULT;
> > > >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int
> > > > flags)
> > > 2 above funcs could be moved along with qemu_open() to osdep.h
> > 
> > I would happily move them, if somebody gave me an one-line description
> > of what's the purpose of osdep.h (so I know what is supposed to be
> > there, and what's not supposed to be there).
> they are defined in osdep.c, it's logical to have declarations in osdep.h
> 
> > 
> > > 
> > > > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > > > new file mode 100644
> > > > index 0000000..b2e8eda
> > > > --- /dev/null
> > > > +++ b/qemu-stdio.h
> > > > @@ -0,0 +1,76 @@
> > > > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > > > + */
> > > > +#ifndef QEMU_STDIO_H
> > > > +#define QEMU_STDIO_H
> > > > +
> > > > +#include "compiler.h"
> > > > +#include "config-host.h"
> > > > +
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <fcntl.h>
> > > > +#include <sys/types.h>
> > > > +#include <sys/stat.h>
> > > > +#include <sys/time.h>
> > > > +#include <sys/mman.h>
> > > > +
> > > > +#ifndef O_LARGEFILE
> > > > +#define O_LARGEFILE 0
> > > > +#endif
> > > > +#ifndef O_BINARY
> > > > +#define O_BINARY 0
> > > > +#endif
> > > > +#ifndef MAP_ANONYMOUS
> > > > +#define MAP_ANONYMOUS MAP_ANON
> > > > +#endif
> > > > +#ifndef ENOMEDIUM
> > > > +#define ENOMEDIUM ENODEV
> > > > +#endif
> > > > +#if !defined(ENOTSUP)
> > > > +#define ENOTSUP 4096
> > > > +#endif
> > > > +#if !defined(ECANCELED)
> > > > +#define ECANCELED 4097
> > > > +#endif
> > > > +#ifndef TIME_MAX
> > > > +#define TIME_MAX LONG_MAX
> > > > +#endif
> > > > +
> > > > +#ifndef CONFIG_IOVEC
> > > > +#define CONFIG_IOVEC
> > > > +struct iovec {
> > > > +    void *iov_base;
> > > > +    size_t iov_len;
> > > > +};
> > > > +/*
> > > > + * Use the same value as Linux for now.
> > > > + */
> > > > +#define IOV_MAX     1024
> > > > +#else
> > > > +#include <sys/uio.h>
> > > > +#endif
> > > > +
> > > > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > > +    GCC_FMT_ATTR(2, 3);
> > > > +
> > > > +#ifdef _WIN32
> > > > +#define fsync _commit
> > > > +#if !defined(lseek)
> > > > +# define lseek _lseeki64
> > > > +#endif
> > > > +int qemu_ftruncate64(int, int64_t);
> > > > +#if !defined(ftruncate)
> > > > +# define ftruncate qemu_ftruncate64
> > > > +#endif
> > > > +
> > > > +static inline char *realpath(const char *path, char *resolved_path)
> > > > +{
> > > > +    _fullpath(resolved_path, path, _MAX_PATH);
> > > > +    return resolved_path;
> > > > +}
> > > > +#endif
> > > > +
> > > > +int qemu_open(const char *name, int flags, ...);
> > > > +int qemu_close(int fd);
> > > qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> > > better to move their declaration to osdep.h.
> > > The rest looks like it fits a purpose of osdep.h as well, so probably it
> > > could be moved there and we could avoid creating a new header.
> > 
> > Maybe, but I really would like to have a good description of what's the
> > meaning/purpose of osdep.h, first. osdep.h seems to have lots of
> Judging from commit logs its purpose is in abstracting/unifying OS depended
> features to some common denominator. And the code you are moving is
> exactly does this.
> 
> > unrelated stuff inside it, already, and I don't want to risk making
> > osdep.h become a new catchall header file, like qemu-common.h is today.
> > 
> So far osdep.h is pretty simple in terms of header dependences and doesn't
> include catch-all headers, moving this declarations and defines there
> doesn't complicate it in this aspect.

Sounds good to me. I guess the description you put above makes sure
osdep.h won't start including every single header out there, like
qemu-common.h does.

> 
> > > 
> > > > +
> > > > +#endif /* QEMU_STDIO_H */
> > > 
> > 
>