On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> On Fri, 9 Nov 2012 12:56:34 -0200
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > This will help reduce the qemu-common.h dependency hell.
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@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 */
>
--
Eduardo
