Implemented bufferevent_socket_connect_hostname_hints()

[jspada]

• Implemented bufferevent_socket_connect_hostname_hints so that ai_flags (such as AI_ADDRCONFIG) can be specified.
• Added a simple unit test

[errzey]

Nothing to do with your patch, but this has always bugged me. We do an unnecessary check to make sure it's not negative and is less than 65535.

connect_hostname(..., ..., ..., ...., uint16_t port); /* sanity */

[errzey]

One silly thing I don't like is having an extra jump for a widely used function. Maybe something a little less overhead, for example:

#define bufferevent_socket_connect_hostname(bev, dnsev, family, host, port) \
    ({ int res;                                                             \
       struct addrinfo hint;                                                \
                                                                            \
       memset(&hint, 0, sizeof(hint));                                      \
                                                                            \
       hint.ai_family = family;                                             \
       hint.ai_protocol = IPPROTO_TCP;                                      \
       hint.ai_socktype = SOCK_STREAM;                                      \
                                                                            \
       res = bufferevent_socket_connect_hostname_hints(                     \
           bev, dnsev, &hint, host, port);                                  \
                                                                            \
       res; })

Yes. yes, statement expressions aren't portable with compilers circa 1982, but the point is made.

[azat]

Hi @ellzey,

I don't like the idea with defines (especially for exported API), maybe adding static inline int bufferevent_socket_connect_impl_(...) helper and use it everywhere will be better? (also we could check whether it is inlined by the compiler or not).

[errzey]

@azat inlines are in most cases never really inlined. And we already use statement expressions in a few places aready.

[azat]

@ellzey my main macro cons -- they are not debugging friendly. As for
inline, in gcc/clang we have __attribute__((always_inline)), but does it
worth it, I mean this function is not so hot to make it macro, no?

P.S. firstly I replied via email (but github didn't accept this), so can be dup2

[errzey]

While working on the cmake refactor, I was wondering why the attribute was never taken into account..

The more we use [inlines|statements|macros] we must consider the size of the resulting objects. They grow with each reference.

I've used this in the past without any pre-configure. It can be expanded upon.

#if (__GNUC__ == 4 && __GNUC_MINOR__ >= 3 && defined(__GNUC_STDC_INLINE__) && !defined(C99_INLINE)) || \
    (__APPLE_CC__ > 5400 && !defined(C99_INLINE) && __STDC_VERSION__ >= 199901L)
# define C99_INLINE 1
#endif

#if C99_INLINE || __GNUC_GNU_INLINE__
# define INLINE     inline
# define HAS_INLINE  1
#else
# define INLINE                                            /* no inline */
#endif

[azat]

Applied, thanks!

(conflicts solved, test had been rewritten to avoid copy-paste)