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

db_postgres compilation fails if htonll is defined #665

Closed
sjthomason opened this issue Jun 9, 2016 · 5 comments · Fixed by #682
Closed

db_postgres compilation fails if htonll is defined #665

sjthomason opened this issue Jun 9, 2016 · 5 comments · Fixed by #682

Comments

@sjthomason
Copy link
Contributor


pg_fld.c:97:40: error: expected ‘)’ before ‘in’
 static inline uint64_t htonll(uint64_t in)
                                        ^
pg_fld.c:107:40: error: expected ‘)’ before ‘in’
 static inline uint64_t ntohll(uint64_t in)
                                        ^

There is a fairly specific ifdef that doesn't match other platforms that define htonll and ntohll such as Solaris:

#if !defined(__OS_darwin) || (defined(__OS_darwin) && !defined(NTOHLL))

Would it be acceptable to change these to:

#ifndef htonll

For example:

diff --git a/modules/db_postgres/pg_fld.c b/modules/db_postgres/pg_fld.c
index de62c14..d9b4911 100644
--- a/modules/db_postgres/pg_fld.c
+++ b/modules/db_postgres/pg_fld.c
@@ -93,7 +93,7 @@ union ull {
        uint32_t ui32[2];
 };

-#if !defined(__OS_darwin) || (defined(__OS_darwin) && !defined(NTOHLL))
+#ifndef htonll
 static inline uint64_t htonll(uint64_t in)
 {
        union ull* p = (union ull*)∈
@@ -103,7 +103,7 @@ static inline uint64_t htonll(uint64_t in)
 #endif


-#if !defined(__OS_darwin) || (defined(__OS_darwin) && !defined(NTOHLL))
+#ifndef ntohll
 static inline uint64_t ntohll(uint64_t in)
 {
        union ull* p = (union ull*)∈
@miconda
Copy link
Member

miconda commented Jun 14, 2016

Can one be sure that htonll is a defined macro on all systems? Because the functions cannot be tested with #ifdef.

@sjthomason
Copy link
Contributor Author

sjthomason commented Jun 14, 2016

Hi Daniel,
I guess we can't be 100% sure of ALL platforms but it is a macro on all the ones I tested:

SmartOS and Solaris 11:

#if !defined(_XPG4_2) || defined(__EXTENSIONS__)
#define ntohll(x)       (x)
#define htonll(x)       (x)
#endif  /* !_XPG4_2 || __EXTENSIONS__ */

OSX:

#if     defined(KERNEL) || (!defined(_POSIX_C_SOURCE) || defined(_DARWIN_C_SOURCE))

#define ntohll(x)       ((__uint64_t)(x))
#define htonll(x)       ((__uint64_t)(x))

#define NTOHL(x)        (x)
#define NTOHS(x)        (x)
#define NTOHLL(x)       (x)
#define HTONL(x)        (x)
#define HTONS(x)        (x)
#define HTONLL(x)       (x)
#endif /* defined(KERNEL) || (!defined(_POSIX_C_SOURCE) || defined(_DARWIN_C_SOURCE)) */

@miconda
Copy link
Member

miconda commented Jun 23, 2016

Can you check if NTOHLL is defined on Solaris? It is quite common that a token supposed to be a define flag/macro uses uppercases.

@sjthomason
Copy link
Contributor Author

Hi Daniel,
I must have not had enough coffee... testing the Solaris macros above only work on big endian machines. I've included the relevant parts of sys/byteorder.h below. Would it be acceptable to change these functions to _ntohll and _htonll to avoid the namespace conflict? The challenge with checking __OS_solaris is that this was a relatively recent addition, I think only available in Solaris 11 and the Ilumos derivatives.

#include <sys/isa_defs.h>
#include <sys/int_types.h>

#if defined(__GNUC__) && defined(_ASM_INLINES) && \
        (defined(__i386) || defined(__amd64))
#include <asm/byteorder.h>
#endif

#ifdef  __cplusplus
extern "C" {
#endif

/*
 * macros for conversion between host and (internet) network byte order
 */

#if defined(_BIG_ENDIAN) && !defined(ntohl) && !defined(__lint)
/* big-endian */
#define ntohl(x)        (x)
#define ntohs(x)        (x)
#define htonl(x)        (x)
#define htons(x)        (x)
#if !defined(_XPG4_2) || defined(__EXTENSIONS__)
#define ntohll(x)       (x)
#define htonll(x)       (x)
#endif  /* !_XPG4_2 || __EXTENSIONS__ */

#elif !defined(ntohl) /* little-endian */

#ifndef _IN_PORT_T
#define _IN_PORT_T
typedef uint16_t in_port_t;
#endif

#ifndef _IN_ADDR_T
#define _IN_ADDR_T
typedef uint32_t in_addr_t;
#endif

#if !defined(_XPG4_2) || defined(__EXTENSIONS__) || defined(_XPG5)
extern  uint32_t htonl(uint32_t);
extern  uint16_t htons(uint16_t);
extern  uint32_t ntohl(uint32_t);
extern  uint16_t ntohs(uint16_t);
#else
extern  in_addr_t htonl(in_addr_t);
extern  in_port_t htons(in_port_t);
extern  in_addr_t ntohl(in_addr_t);
extern  in_port_t ntohs(in_port_t);
#endif  /* !_XPG4_2 || __EXTENSIONS__ || _XPG5 */

#if defined(_LP64) || defined(_LONGLONG_TYPE)
#if !defined(_XPG4_2) || defined(__EXTENSIONS__)
extern  uint64_t htonll(uint64_t);
extern  uint64_t ntohll(uint64_t);
#endif  /* !_XPG4_2 || __EXTENSIONS__ */
#endif  /* _LP64 || _LONGLONG_TYPE  */
#endif

@miconda
Copy link
Member

miconda commented Jun 23, 2016

I am fine to rename the functions to avoid the conflict. make a pull request for it - there is CI build to see if there are compilations issues with PRs when using gcc or clang on linux.

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

Successfully merging a pull request may close this issue.

2 participants