Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails under illumos + epoll gaps? #257

Open
faithanalog opened this issue Mar 5, 2024 · 1 comment
Open

Build fails under illumos + epoll gaps? #257

faithanalog opened this issue Mar 5, 2024 · 1 comment

Comments

@faithanalog
Copy link

faithanalog commented Mar 5, 2024

Currently, this project fails to build on illumos:

gcc -O2 -std=gnu99 -fPIC -g -Wall -Wextra  -Wno-missing-field-initializers  -Wno-override-init -Wno-unused  -DLUA_COMPAT_APIINTCASTS -I/opt/pkg/include/lua-5.4 -D_REENTRANT -D_THREAD_SAFE -D_GNU_SOURCE -Usun -D_XPG4_2 -D__EXTENSIONS__  -DCOMPAT53_PREFIX=cqueues -DCQUEUES_VENDOR='"[email protected]"' -DCQUEUES_VERSION=20200726L -DCQUEUES_COMMIT='"b0c2bc5979f008a39300b01a0dc3e967c6257be9"' -c -o /home/vi/z/cqueues2/src/5.4/cqueues.o /home/vi/z/cqueues2/src/cqueues.c
/home/vi/z/cqueues2/src/cqueues.c: In function 'kpoll_alert':
/home/vi/z/cqueues2/src/cqueues.c:688:18: warning: implicit declaration of function 'port_send' [-Wimplicit-function-declaration]
  688 |         if (0 != port_send(kp->fd, POLLIN, &kp->alert)) {
      |                  ^~~~~~~~~
/home/vi/z/cqueues2/src/cqueues.c: In function 'kpoll_isalert':
/home/vi/z/cqueues2/src/cqueues.c:762:21: error: 'kpoll_event_t' {aka 'const struct epoll_event'} has no member named 'portev_source'
  762 |         return event->portev_source == PORT_SOURCE_USER;
      |                     ^~
/home/vi/z/cqueues2/src/cqueues.c:762:40: error: 'PORT_SOURCE_USER' undeclared (first use in this function)
  762 |         return event->portev_source == PORT_SOURCE_USER;
      |                                        ^~~~~~~~~~~~~~~~
/home/vi/z/cqueues2/src/cqueues.c:762:40: note: each undeclared identifier is reported only once for each function it appears in
/home/vi/z/cqueues2/src/cqueues.c:766:1: warning: control reaches end of non-void function [-Wreturn-type]
  766 | } /* kpoll_isalert() */
      | ^

These errors appear to be because while ENABLE_EPOLL is defined, so is ENABLE_PORTS. But, because ENABLE_EPOLL is defined, ports.h is never included.

Now that said, I think that some of these port usages may be unintentional, since it seems like the intent is to use epoll instead of ports when available.

I was able to get the code to compile and work with epoll, with this patch, applied to b0c2bc5 (master as of writing):

diff --git a/src/cqueues.c b/src/cqueues.c
index c9d6299..2bd1e5d 100644
--- a/src/cqueues.c
+++ b/src/cqueues.c
@@ -478,7 +478,17 @@ static int kpoll_ctl(struct kpoll *, int, short *, short, void *);
 static int alert_rearm(struct kpoll *);
 
 static int alert_init(struct kpoll *kp) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+	int error;
+
+	if (kp->alert.fd[0] != -1)
+		return 0;
+
+	if ((error = cqs_pipe(kp->alert.fd, O_CLOEXEC|O_NONBLOCK)))
+		return error;
+
+	return alert_rearm(kp);
+#elif ENABLE_PORTS
 	(void)kp;
 	return 0;
 #elif HAVE_EVENTFD
@@ -503,7 +513,9 @@ static int alert_init(struct kpoll *kp) {
 } /* alert_init() */
 
 static void alert_destroy(struct kpoll *kp, int (*closefd)(int *, void *), void *cb_udata) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+	(void)kp;
+#elif ENABLE_PORTS
 	(void)kp;
 #else
 	for (size_t i = 0; i < countof(kp->alert.fd); i++)
@@ -512,7 +524,9 @@ static void alert_destroy(struct kpoll *kp, int (*closefd)(int *, void *), void
 } /* alert_destroy() */
 
 static int alert_rearm(struct kpoll *kp) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+	return kpoll_ctl(kp, kp->alert.fd[0], &kp->alert.state, POLLIN, &kp->alert);
+#elif ENABLE_PORTS
 	return 0;
 #else
 	return kpoll_ctl(kp, kp->alert.fd[0], &kp->alert.state, POLLIN, &kp->alert);
@@ -583,7 +597,10 @@ static inline short kpoll_pending(const kpoll_event_t *event) {
 
 
 static inline short kpoll_diff(const kpoll_event_t *event NOTUSED, short ostate NOTUSED) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+	// XXX is this ok?
+	return ostate;
+#elif ENABLE_PORTS
 	/* Solaris Event Ports aren't persistent. */
 	return 0;
 #else
@@ -684,7 +701,8 @@ static int kpoll_alert(struct kpoll *kp) {
 	/* initialization may have been delayed */
 	if ((error = alert_init(kp)))
 		return error;
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+#elif ENABLE_PORTS
 	if (0 != port_send(kp->fd, POLLIN, &kp->alert)) {
 		if (errno != EBUSY)
 			return errno;
@@ -720,7 +738,8 @@ static int kpoll_alert(struct kpoll *kp) {
 static int kpoll_calm(struct kpoll *kp) {
 	int error;
 
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+#elif ENABLE_PORTS
 	/* each PORT_SOURCE_USER event is discrete */
 #elif HAVE_EVENTFD
 	uint64_t n;
@@ -758,7 +777,9 @@ static int kpoll_calm(struct kpoll *kp) {
 
 
 static inline short kpoll_isalert(struct kpoll *kp, const kpoll_event_t *event) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+	return kpoll_udata(event) == &kp->alert;
+#elif ENABLE_PORTS
 	return event->portev_source == PORT_SOURCE_USER;
 #else
 	return kpoll_udata(event) == &kp->alert;

I do not actually know if I am doing things correctly, because I have never written code that uses epoll or ports directly. But this does make make lua-http from https://github.com/daurnimator/lua-http run correctly on my illumos machine (OmniOS-based distribution).

Alternatively, simply disabling EPOLL and using ports instead also worked:

diff --git a/src/cqueues.h b/src/cqueues.h
index ef803ea..d0c37a4 100644
--- a/src/cqueues.h
+++ b/src/cqueues.h
@@ -74,10 +74,6 @@
 
 #define UCLIBC_PREREQ(M, m, p) (defined __UCLIBC__ && (__UCLIBC_MAJOR__ > M || (__UCLIBC_MAJOR__ == M && __UCLIBC_MINOR__ > m) || (__UCLIBC_MAJOR__ == M && __UCLIBC_MINOR__ == m && __UCLIBC_SUBLEVEL__ >= p)))
 
-#ifndef ENABLE_EPOLL
-#define ENABLE_EPOLL HAVE_EPOLL_CREATE
-#endif
-
 #ifndef ENABLE_PORTS
 #define ENABLE_PORTS HAVE_PORT_CREATE
 #endif
@daurnimator
Copy link
Collaborator

Now that said, I think that some of these port usages may be unintentional, since it seems like the intent is to use epoll instead of ports when available.

At least when we wrote it, there was no epoll on illumos and ports was the only option. I don't think we ever considered a target/day when there would be both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants