Patchwork : Fix conditional compilation for OpenBSD

login
register
mail settings
Submitter Michael W. Bombardieri
Date March 31, 2013, 12:46 p.m.
Message ID <20130331124656.GA565@bom.nom.co>
Download mbox | patch
Permalink /patch/232607/
State New
Headers show

Comments

Michael W. Bombardieri - March 31, 2013, 12:46 p.m.
On Sun, Mar 31, 2013 at 12:48:04AM +0000, Peter Maydell wrote:
> 
> Hi Michael; thanks for the patch. However I think we should
> fix this by having a configure check for sem_timedwait [or
> whatever the functions we need are], rather than by piling
> up another OS ifdef check.
> 
> (If you want to have a go at that it would probably be
> useful to read the wiki.qemu.org page on submitting
> patches; in particular without a Signed-off-by: line
> we can't apply a patch at all.)
> 
> -- PMM

What about this?

Re-use the exting the pthread checker code in 'configure', define HAS_PTHREAD
and use this instead of OS specific #if checks. I have built this on

1. Ubuntu 10.04.3 LTS (Linux 2.6.32-36-generic i686)
2. OpenBSD 5.1 (OpenBSD 5.1 GENERIC.MP#188 i386)
3. OpenBSD 5.1 (OpenBSD 5.1 GENERIC#243 amd64)

- Michael
Peter Maydell - March 31, 2013, 1:36 p.m.
On 31 March 2013 13:46, Michael W. Bombardieri <mb@ii.net> wrote:
> What about this?
>
> Re-use the exting the pthread checker code in 'configure', define HAS_PTHREAD
> and use this instead of OS specific #if checks. I have built this on
>
> 1. Ubuntu 10.04.3 LTS (Linux 2.6.32-36-generic i686)
> 2. OpenBSD 5.1 (OpenBSD 5.1 GENERIC.MP#188 i386)
> 3. OpenBSD 5.1 (OpenBSD 5.1 GENERIC#243 amd64)

This doesn't look right, because it means we'll use the fallback
code even for OSes like Linux which do have the ability to use
the (preferable) semaphore code. You want a config check for
sem_timedwait, which will succeed on Linux and fail on the BSDs,
and then instead of #if defined(various BSDs) you have
#ifndef CONFIG_SEM_TIMEDWAIT.

> +if [ $pthread = 'yes' ]; then
> +  QEMU_CFLAGS="-DHAS_PTHREAD $QEMU_CFLAGS"
> +fi
> +

Have a look in configure at how we handle the preadv()
check as an example. There's a little test program
which is compiled to see if the function exists, and
we set a shell variable. Then later on we write CONFIG_PREADV=y
to the config-host.mak file. This avoids enormous compiler
command lines for every config check.

Also you still aren't providing a Signed-off-by: line for
your patch, and it would be nice if you could submit
it in the git format-patch format that makes it trivial
to apply.

thanks
-- PMM

Patch

diff --git a/configure b/configure
index f2af714..2e521e2 100755
--- a/configure
+++ b/configure
@@ -2296,6 +2296,10 @@  else
   done
 fi
 
+if [ $pthread = 'yes' ]; then
+  QEMU_CFLAGS="-DHAS_PTHREAD $QEMU_CFLAGS"
+fi
+
 if test "$mingw32" != yes -a "$pthread" = no; then
   echo
   echo "Error: pthread check failed"
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index 0f30dcc..f547f29 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -12,7 +12,7 @@  struct QemuCond {
 };
 
 struct QemuSemaphore {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_t lock;
     pthread_cond_t cond;
     int count;
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 4489abf..edf4715 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -122,7 +122,7 @@  void qemu_sem_init(QemuSemaphore *sem, int init)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     rc = pthread_mutex_init(&sem->lock, NULL);
     if (rc != 0) {
         error_exit(rc, __func__);
@@ -147,7 +147,7 @@  void qemu_sem_destroy(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     rc = pthread_cond_destroy(&sem->cond);
     if (rc < 0) {
         error_exit(rc, __func__);
@@ -168,7 +168,7 @@  void qemu_sem_post(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_lock(&sem->lock);
     if (sem->count == INT_MAX) {
         rc = EINVAL;
@@ -206,7 +206,7 @@  int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
     int rc;
     struct timespec ts;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     compute_abs_deadline(&ts, ms);
     pthread_mutex_lock(&sem->lock);
     --sem->count;
@@ -249,7 +249,7 @@  int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
 
 void qemu_sem_wait(QemuSemaphore *sem)
 {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_lock(&sem->lock);
     --sem->count;
     while (sem->count < 0) {