diff mbox

[2/2,RESENT-INLINE] Remove unnecessary CONFIG_EVENTFD preprocessor conditional to satisfy link

Message ID 1462237265-61763-3-git-send-email-chrisfriedt@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christopher Friedt May 3, 2016, 1:01 a.m. UTC
The file ivshmem.c unconditionally references event_notifier_init_fd() in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined. On platforms where CONFIG_POSIX is defined, but CONFIG_EVENTFD is not defined, that results in an undefined symbol referenced from ivshmem.c and the link fails. That applies to Mac OS X, but possibly other BSD-based distros.

Note: there is nothing specific to eventfd inside and event_notifier_init() also fails unconditionally if CONFIG_EVENTFD is not defined.
Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
---
 util/event_notifier-posix.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Markus Armbruster May 3, 2016, 6:59 a.m. UTC | #1
Christopher Friedt <chrisfriedt@gmail.com> writes:

> The file ivshmem.c unconditionally references event_notifier_init_fd() in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined. On platforms where CONFIG_POSIX is defined, but CONFIG_EVENTFD is not defined, that results in an undefined symbol referenced from ivshmem.c and the link fails. That applies to Mac OS X, but possibly other BSD-based distros.

ivshmem.c cannot work without CONFIG_EVENTFD, and therefore is not
compiled without it.

A link error for event_notifier_init_fd() from ivshmem.o indicates you
got CONFIG_EVENTFD=y for make (since ivshmem.o gets linked), but
!defined(CONFIG_EVENTFD) for C (or else event_notifier_init_fd() would
exist).  Your build tree is messed up, or the makefiles are broken.  Try
starting over with a fresh build tree.

See also
http://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg01437.html

> Note: there is nothing specific to eventfd inside and event_notifier_init() also fails unconditionally if CONFIG_EVENTFD is not defined.

I'm afraid you're misreading the code.

event_notifier_init() has a working fallback: if the host doesn't
support eventfd (!defined(CONFIG_EVENTFD) or eventfd() fails with
ENOSYS), then we emulate eventfd with a pair of pipes.  Possible only
because pipes have special properties.

To support the emulation, we have two file descriptors, @rfd for reading
and @wfd for writing.  Design invariant: if they are the same file
descriptor, it must be a proper eventfd, and if they are different, they
must be a pair of pipes.  The only way to establish the invariant with
event_notifier_init_fd() is passing it an eventfd, as the function's
contract says.  Without CONFIG_EVENTFD, there is no way to use the
function correctly, so defining it would be nothing but a trap.

See also commit 330b583, which added the ifdef.

> Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
> ---
>  util/event_notifier-posix.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
> index c1f0d79..c9bb34d 100644
> --- a/util/event_notifier-posix.c
> +++ b/util/event_notifier-posix.c
> @@ -21,7 +21,6 @@
>  #include <sys/eventfd.h>
>  #endif
>  
> -#ifdef CONFIG_EVENTFD
>  /*
>   * Initialize @e with existing file descriptor @fd.
>   * @fd must be a genuine eventfd object, emulation with pipe won't do.
> @@ -31,7 +30,6 @@ void event_notifier_init_fd(EventNotifier *e, int fd)
>      e->rfd = fd;
>      e->wfd = fd;
>  }
> -#endif
>  
>  int event_notifier_init(EventNotifier *e, int active)
>  {

NAK
diff mbox

Patch

diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index c1f0d79..c9bb34d 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -21,7 +21,6 @@ 
 #include <sys/eventfd.h>
 #endif
 
-#ifdef CONFIG_EVENTFD
 /*
  * Initialize @e with existing file descriptor @fd.
  * @fd must be a genuine eventfd object, emulation with pipe won't do.
@@ -31,7 +30,6 @@  void event_notifier_init_fd(EventNotifier *e, int fd)
     e->rfd = fd;
     e->wfd = fd;
 }
-#endif
 
 int event_notifier_init(EventNotifier *e, int active)
 {