Hi Alexander,
Am 24.03.2015 um 00:36 schrieb Alexander von Gluck IV:
> * skip syscall.h on Haiku
> * skip signal.h on Haiku
> * no daemon function
> * only attach SIGIO when it exists
> * use termios.h on Haiku
We cannot accept patches without Signed-off-by, indicating that this
code is either from you or someone who has signed it off before you
according to the DCO.
That is, if you took this patch from HaikuPorts, you'll need to track
down who wrote which parts of this and get them to sign off before
resubmitting here. Michael Lotz never responded after posting the
previous big patch.
> ---
> main-loop.c | 2 +
> os-posix.c | 4 ++
> target-xtensa/xtensa-semi.c | 84
> ++++++++++++++++++++-----------------------
> tests/Makefile | 2 +-
> util/compatfd.c | 2 +
> util/oslib-posix.c | 7 ++++
> util/qemu-openpty.c | 4 ++-
> 7 files changed, 58 insertions(+), 47 deletions(-)
Can you split this up into a series of smaller patches? That'll make it
easier to start applying fixes.
You also forgot to CC the Xtensa maintainer whose code you are touching.
>
> diff --git a/main-loop.c b/main-loop.c
> index 981bcb5..947d7cd 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -81,7 +81,9 @@ static int qemu_signal_init(void)
> */
> sigemptyset(&set);
> sigaddset(&set, SIG_IPI);
> + #ifdef SIGIO
Unless there is precedent (which I doubt), do not indent #ifdefs beyond
recognition.
> sigaddset(&set, SIGIO);
> + #endif
> sigaddset(&set, SIGALRM);
> sigaddset(&set, SIGBUS);
> /* SIGINT cannot be handled via signalfd, so that ^C can be used
Otherwise this hunk looks okay.
> diff --git a/os-posix.c b/os-posix.c
> index ba091f1..43f7fec 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -323,6 +323,9 @@ bool is_daemonized(void)
>
> int os_mlock(void)
> {
> +#if defined(CONFIG_HAIKU)
> + return ENOSYS;
So Haiku does not implement mlock*? That should be explained in the
commit message.
Are you sure that ENOSYS is correct here as opposed to -ENOSYS? :)
BeOS used negative E* codes, POSIX uses positive ones - or the other way
around - and to complicate the matter further, Ingo had written some
Haiku library hack to translate between both versions...
> +#else
> int ret = 0;
>
> ret = mlockall(MCL_CURRENT | MCL_FUTURE);
> @@ -331,4 +334,5 @@ int os_mlock(void)
> }
>
> return ret;
> +#endif
> }
> diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c
> index 16e9d8c..d0ea12a 100644
> --- a/target-xtensa/xtensa-semi.c
> +++ b/target-xtensa/xtensa-semi.c
> @@ -95,59 +95,53 @@ enum {
>
> static uint32_t errno_h2g(int host_errno)
> {
> - static const uint32_t guest_errno[] = {
> - [EPERM] = TARGET_EPERM,
> - [ENOENT] = TARGET_ENOENT,
> - [ESRCH] = TARGET_ESRCH,
> - [EINTR] = TARGET_EINTR,
> - [EIO] = TARGET_EIO,
> - [ENXIO] = TARGET_ENXIO,
> - [E2BIG] = TARGET_E2BIG,
> - [ENOEXEC] = TARGET_ENOEXEC,
> - [EBADF] = TARGET_EBADF,
> - [ECHILD] = TARGET_ECHILD,
> - [EAGAIN] = TARGET_EAGAIN,
> - [ENOMEM] = TARGET_ENOMEM,
> - [EACCES] = TARGET_EACCES,
> - [EFAULT] = TARGET_EFAULT,
> + switch (host_errno) {
> + case 0: return 0;
> + case EPERM: return TARGET_EPERM;
> + case ENOENT: return TARGET_ENOENT;
> + case ESRCH: return TARGET_ESRCH;
> + case EINTR: return TARGET_EINTR;
> + case EIO: return TARGET_EIO;
> + case ENXIO: return TARGET_ENXIO;
> + case E2BIG: return TARGET_E2BIG;
> + case ENOEXEC: return TARGET_ENOEXEC;
> + case EBADF: return TARGET_EBADF;
> + case ECHILD: return TARGET_ECHILD;
> + case EAGAIN: return TARGET_EAGAIN;
> + case ENOMEM: return TARGET_ENOMEM;
> + case EACCES: return TARGET_EACCES;
> + case EFAULT: return TARGET_EFAULT;
> #ifdef ENOTBLK
> - [ENOTBLK] = TARGET_ENOTBLK,
> + case ENOTBLK: return TARGET_ENOTBLK;
> #endif
> - [EBUSY] = TARGET_EBUSY,
> - [EEXIST] = TARGET_EEXIST,
> - [EXDEV] = TARGET_EXDEV,
> - [ENODEV] = TARGET_ENODEV,
> - [ENOTDIR] = TARGET_ENOTDIR,
> - [EISDIR] = TARGET_EISDIR,
> - [EINVAL] = TARGET_EINVAL,
> - [ENFILE] = TARGET_ENFILE,
> - [EMFILE] = TARGET_EMFILE,
> - [ENOTTY] = TARGET_ENOTTY,
> + case EBUSY: return TARGET_EBUSY;
> + case EEXIST: return TARGET_EEXIST;
> + case EXDEV: return TARGET_EXDEV;
> + case ENODEV: return TARGET_ENODEV;
> + case ENOTDIR: return TARGET_ENOTDIR;
> + case EISDIR: return TARGET_EISDIR;
> + case EINVAL: return TARGET_EINVAL;
> + case ENFILE: return TARGET_ENFILE;
> + case EMFILE: return TARGET_EMFILE;
> + case ENOTTY: return TARGET_ENOTTY;
Why convert from array to switch statement? It looks like a very
invasive change for no obvious reason.
> #ifdef ETXTBSY
> - [ETXTBSY] = TARGET_ETXTBSY,
> + case ETXTBSY: return TARGET_ETXTBSY;
> #endif
> - [EFBIG] = TARGET_EFBIG,
> - [ENOSPC] = TARGET_ENOSPC,
> - [ESPIPE] = TARGET_ESPIPE,
> - [EROFS] = TARGET_EROFS,
> - [EMLINK] = TARGET_EMLINK,
> - [EPIPE] = TARGET_EPIPE,
> - [EDOM] = TARGET_EDOM,
> - [ERANGE] = TARGET_ERANGE,
> - [ENOSYS] = TARGET_ENOSYS,
> + case EFBIG: return TARGET_EFBIG;
> + case ENOSPC: return TARGET_ENOSPC;
> + case ESPIPE: return TARGET_ESPIPE;
> + case EROFS: return TARGET_EROFS;
> + case EMLINK: return TARGET_EMLINK;
> + case EPIPE: return TARGET_EPIPE;
> + case EDOM: return TARGET_EDOM;
> + case ERANGE: return TARGET_ERANGE;
> + case ENOSYS: return TARGET_ENOSYS;
> #ifdef ELOOP
> - [ELOOP] = TARGET_ELOOP,
> + case ELOOP: return TARGET_ELOOP;
> #endif
> };
>
> - if (host_errno == 0) {
> - return 0;
> - } else if (host_errno > 0 && host_errno < ARRAY_SIZE(guest_errno) &&
> - guest_errno[host_errno]) {
> - return guest_errno[host_errno];
> - } else {
> - return TARGET_EINVAL;
> - }
> + return TARGET_EINVAL;
> }
>
> void HELPER(simcall)(CPUXtensaState *env)
> diff --git a/tests/Makefile b/tests/Makefile
> index 55aa745..a28e9fe 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -370,7 +370,7 @@ tests/qemu-iotests/socket_scm_helper$(EXESUF):
> tests/qemu-iotests/socket_scm_hel
> tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a
> libqemustub.a
> tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o
> $(block-obj-y) libqemuutil.a libqemustub.a
>
> -ifeq ($(CONFIG_POSIX),y)
> +ifeq ($(CONFIG_POSIX)$(CONFIG_HAIKU),yn)
> LIBS += -lutil
> endif
>
Did you test this on a Linux or other non-Haiku system?
> diff --git a/util/compatfd.c b/util/compatfd.c
> index 341ada6..53253c7 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -17,7 +17,9 @@
> #include "qemu/compatfd.h"
> #include "qemu/thread.h"
>
> +#ifndef CONFIG_HAIKU
> #include <sys/syscall.h>
> +#endif
>
> struct sigfd_compat_info
> {
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 37ffd96..bc20a57 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -60,7 +60,10 @@ extern int daemon(int, int);
> #include <sys/mman.h>
> #include <libgen.h>
> #include <setjmp.h>
> +
> +#ifndef CONFIG_HAIKU
> #include <sys/signal.h>
> +#endif
>
> #ifdef CONFIG_LINUX
> #include <sys/syscall.h>
Here sys/syscall.h is guarded by CONFIG_LINUX - I wonder whether that
was intended in compatfd.c above, too? (not sure about Mac OS X and BSD)
> @@ -82,7 +85,11 @@ int qemu_get_thread_id(void)
>
> int qemu_daemon(int nochdir, int noclose)
> {
> + #ifndef CONFIG_HAIKU
Ditto, don't indent this way please.
> return daemon(nochdir, noclose);
> + #else
> + return -1;
> + #endif
> }
>
> void *qemu_oom_check(void *ptr)
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 4c53211..d7c43c4 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -35,7 +35,9 @@
> #include "config-host.h"
> #include "qemu-common.h"
>
> -#if defined(__GLIBC__)
> +#if defined(__HAIKU__)
> +# include <termios.h>
> +#elif defined(__GLIBC__)
> # include <pty.h>
> #elif defined CONFIG_BSD
> # include <termios.h>
This hunk seems sensible - Haiku uses parts of glibc (commit message!).
I wonder whether we should combine that with CONFIG_BSD though, moving
it up?
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
