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

Compilation fails on AIX with 7.55.1 #1828

Closed
MikaelSmith opened this Issue Aug 24, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@MikaelSmith

MikaelSmith commented Aug 24, 2017

I did this

I attempted to build curl 7.55.1 on AIX 7.1 using GCC 5.2.0.

I expected the following

I expected it to succeed, as we had previously built 7.51.0 successfully.

curl/libcurl version

7.55.1 (doesn't build, so no curl -V output)

operating system

AIX 7.1

Details

Compilation fails on multi.c. The root of it is that on AIX, when _ALL_SOURCE is set to 1 (as CURL does), sys/poll.h uses #define for event and revent. Because multi.c includes sys/poll.h but multi.h doesn't, we end up with a mismatch between names for the curl_waitfd struct.

libtool: compile:  gcc -DHAVE_CONFIG_H -I../include -I../lib -I../lib -DBUILDING_LIBCURL -isystem /opt/puppetlabs/puppet/include -isystem /opt/pl-build-tools/include -I/opt/puppetlabs/puppet/include -I/opt/puppetlabs/puppet/include/openssl -Werror-implicit-function-declaration -O2 -Wno-system-headers -pthread -MT libcurl_la-multi.lo -MD -MP -MF .deps/libcurl_la-multi.Tpo -c multi.c  -fPIC -DPIC -o .libs/libcurl_la-multi.o
multi.c: In function 'curl_multi_wait':
multi.c:1076:20: error: 'struct curl_waitfd' has no member named 'reqevents'
     if(extra_fds[i].events & CURL_WAIT_POLLIN)
                    ^
multi.c:1078:20: error: 'struct curl_waitfd' has no member named 'reqevents'
     if(extra_fds[i].events & CURL_WAIT_POLLPRI)
                    ^
multi.c:1080:20: error: 'struct curl_waitfd' has no member named 'reqevents'
     if(extra_fds[i].events & CURL_WAIT_POLLOUT)
                    ^
multi.c:1108:21: error: 'struct curl_waitfd' has no member named 'rtnevents'
         extra_fds[i].revents = mask;
                     ^
gmake: *** [libcurl_la-multi.lo] Error 1

Previously this appears to have built with curl 7.51.0 because multi.h also included sys/poll.h, so they were equally wrong. Based on https://curl.haxx.se/mail/lib-2013-02/0330.html, this appears to have been an issue off-and-on for years.

My workaround has been to undef _ALL_SOURCE before including select.h in multi.c, and redefine it after. This probably isn't a good general solution.

MikaelSmith added a commit to MikaelSmith/puppet-agent that referenced this issue Aug 24, 2017

(PA-1447) Patch for AIX builds
An issue with building curl on AIX - that appears to have been a
long-standing latent bug - surfaced in curl 7.55.1. Add a patch to
work-around the issue.

Issue has been filed at curl/curl#1828.

MikaelSmith added a commit to MikaelSmith/puppet-agent that referenced this issue Aug 24, 2017

(PA-1447) Patch for AIX builds
An issue with building curl on AIX - that appears to have been a
long-standing latent bug - surfaced in curl 7.55.1. Add a patch to
work-around the issue.

Issue has been filed at curl/curl#1828.

@bagder bagder added the build label Aug 25, 2017

@bagder

This comment has been minimized.

Member

bagder commented Aug 25, 2017

Undefining _ALL_SOURCE seems a bit rough (that define is added by configure/autoconf btw).

How about making sure sys/poll.h is included before multi.h is included like this patch? (curl_setup.h is the first file multi.c includes)

diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 36d1e42bc..6ed230ac2 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -326,10 +326,14 @@
 
 #ifndef STDC_HEADERS /* no standard C headers! */
 #include <curl/stdcheaders.h>
 #endif
 
+#ifdef _AIX
+#include <sys/poll.h>
+#endif
+
 #ifdef __POCC__
 #  include <sys/types.h>
 #  include <unistd.h>
 #  define sys_nerr EILSEQ
 #endif
@MikaelSmith

This comment has been minimized.

MikaelSmith commented Aug 25, 2017

That would mess up anyone trying to use curl_waitfd outside multi.h. Probably not a problem in my code, but didn't seem like a great solution.

MikaelSmith added a commit to MikaelSmith/puppet-agent that referenced this issue Aug 25, 2017

(PA-1447) Patch for AIX builds
An issue with building curl on AIX - that appears to have been a
long-standing latent bug - surfaced in curl 7.55.1. Add a patch to
work-around the issue.

Issue has been filed at curl/curl#1828.
@bagder

This comment has been minimized.

Member

bagder commented Aug 25, 2017

That would mess up anyone trying to use curl_waitfd outside multi.h

Exactly how would that mess anything up for anyone?

I would agree that it doesn't really help applications that want to use curl_waitfd, as they would have to include <sys/poll.h> on their own before multi.h to really fix this.

A better fix that also works for other applications could be to make sure <curl/system.h> includes it. What do you think?

diff --git a/include/curl/system.h b/include/curl/system.h
index a6640ba52..39dae754c 100644
--- a/include/curl/system.h
+++ b/include/curl/system.h
@@ -375,10 +375,16 @@
 # define CURL_SUFFIX_CURL_OFF_T     L
 # define CURL_SUFFIX_CURL_OFF_TU    UL
 # define CURL_TYPEOF_CURL_SOCKLEN_T int
 #endif
 
+#ifdef _AIX
+/* AIX needs <sys/poll.h> */
+#define CURL_PULL_SYS_POLL_H
+#endif
+
+
 /* CURL_PULL_WS2TCPIP_H is defined above when inclusion of header file  */
 /* ws2tcpip.h is required here to properly make type definitions below. */
 #ifdef CURL_PULL_WS2TCPIP_H
 #  include <winsock2.h>
 #  include <windows.h>
@@ -395,10 +401,16 @@
 /* sys/socket.h is required here to properly make type definitions below. */
 #ifdef CURL_PULL_SYS_SOCKET_H
 #  include <sys/socket.h>
 #endif
 
+/* CURL_PULL_SYS_POLL_H is defined above when inclusion of header file    */
+/* sys/poll.h is required here to properly make type definitions below.   */
+#ifdef CURL_PULL_SYS_POLL_H
+#  include <sys/poll.h>
+#endif
+
 /* Data type definition of curl_socklen_t. */
 #ifdef CURL_TYPEOF_CURL_SOCKLEN_T
   typedef CURL_TYPEOF_CURL_SOCKLEN_T curl_socklen_t;
 #endif
 
@MikaelSmith

This comment has been minimized.

MikaelSmith commented Aug 25, 2017

Yes, having to include <sys/poll.h> before using <multi.h> is what I meant by "mess up". It would likely be unintuitive.

I guess including it for anyone using curl is the a reasonable solution.

bagder added a commit that referenced this issue Aug 25, 2017

system.h: include sys/poll.h for AIX
... to get the event/revent defines that might be used for the poll
struct.

Reported-by: Michael Smith
Fixes #1828

@bagder bagder closed this in 8a84fcc Aug 27, 2017

@bagder

This comment has been minimized.

Member

bagder commented Aug 27, 2017

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.