[PATCH] D98581: [llvm-jitlink] Add diagnostic output and port executor to getaddrinfo(3) as well

[Rafik Zurob via Phabricator via llvm-commits]

rzurob accepted this revision.
rzurob added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/tools/llvm-jitlink/llvm-jitlink-executor/llvm-jitlink-executor.cpp:93
+  outs() << "Listening at " << Host << ":" << PortStr << "\n";
+  return accept(SockFD, AI->ai_addr, &AI->ai_addrlen);
 #endif
----------------
sgraenitz wrote:
> rzurob wrote:
> > This line fails compilation on AIX in 64-bit mode:
> > 
> > ```
> > llvm/tools/llvm-jitlink/llvm-jitlink-executor/llvm-jitlink-executor.cpp:93:10: error: no matching function for call to 'accept'
> >   return accept(SockFD, AI->ai_addr, &AI->ai_addrlen);
> >          ^~~~~~
> > /usr/include/sys/socket.h:635:9: note: candidate function not viable: no known conversion from 'size_t *' (aka 'unsigned long *') to 'socklen_t *' (aka 'unsigned int *') for 3rd argument
> > int     accept(int, struct sockaddr *__restrict__, socklen_t *__restrict__);
> >         ^
> > 1 error generated.
> > Error while processing llvm/tools/llvm-jitlink/llvm-jitlink-executor/llvm-jitlink-executor.cpp.
> > ```
> > 
> > The size is always going to fit into an unsigned int.  So perhaps we need to store AI->ai_addrlen into a socklen_t temp first?
> Interesting, the ai_addrlen field should be of type socklen_t naturally:
> ```
> struct addrinfo {
>   int              ai_flags;
>   int              ai_family;
>   int              ai_socktype;
>   int              ai_protocol;
>   socklen_t        ai_addrlen;
>   struct sockaddr *ai_addr;
>   char            *ai_canonname;
>   struct addrinfo *ai_next;
> };
> ```
> 
> Do you think it's fine to keep this as a special-case for AIX? Does the `connect` call on the client side in D98579 suffer from the same issue?
You're right that the AIX header is not declaring ai_addrlen as what POSIX says.  I've reported it to them.
In the meantime, I confirmed that the AIX workaround you added works.  Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98581/new/

https://reviews.llvm.org/D98581