From 4f261c5b23d2eb3b01da4b894a1d5eabc48f6817 Mon Sep 17 00:00:00 2001 From: Mark Slee Date: Fri, 13 Apr 2007 00:33:24 +0000 Subject: [PATCH] Crazy byteswapping thrift patches from david reiss Reviewed By: ninjitsu Test Plan: bswap64, holla holla git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665098 13f79535-47bb-0310-9956-ffa450edef68 --- CONTRIBUTORS | 8 +++ lib/cpp/configure.ac | 2 + lib/cpp/src/TLogging.h | 11 ++++ lib/cpp/src/Thrift.h | 6 ++ lib/cpp/src/protocol/TBinaryProtocol.cpp | 76 +++++++++++++++++------- lib/cpp/src/protocol/TProtocol.h | 22 +++++-- lib/cpp/src/transport/TFileTransport.cpp | 3 + 7 files changed, 102 insertions(+), 26 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index cd5901f2..0d92a255 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -2,6 +2,14 @@ Release 20070401 ---------------- +David Reiss +-strings.h include fix for bzero +-endianness fixes on TBinaryProtocol double serialization +-improved ntohll,htonll implementation + +Dan Li +-Java TestServer and Tutorial Fixes + Kevin Ko -Fix for unnecessary std::string copy construction in Protocol/Exception diff --git a/lib/cpp/configure.ac b/lib/cpp/configure.ac index 98020963..a6d61e8c 100644 --- a/lib/cpp/configure.ac +++ b/lib/cpp/configure.ac @@ -54,6 +54,8 @@ AC_CHECK_HEADERS([sys/time.h]) AC_CHECK_HEADERS([unistd.h]) +AC_CHECK_HEADERS([endian.h]) + AC_C_INLINE AX_BOOST_BASE([1.33.1]) diff --git a/lib/cpp/src/TLogging.h b/lib/cpp/src/TLogging.h index e814e417..15c9c06b 100644 --- a/lib/cpp/src/TLogging.h +++ b/lib/cpp/src/TLogging.h @@ -7,14 +7,25 @@ #ifndef _THRIFT_LOGGING_H #define _THRIFT_LOGGING_H 1 +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + /** * Contains utility macros for debugging and logging. * * @author Aditya Agarwal */ +#ifndef HAVE_CLOCK_GETTIME #include +#else +#include +#endif + +#ifdef HAVE_STDINT_H #include +#endif /** * T_GLOBAL_DEBUGGING_LEVEL = 0: all debugging turned off, debug macros undefined diff --git a/lib/cpp/src/Thrift.h b/lib/cpp/src/Thrift.h index d024ad58..2cb656b2 100644 --- a/lib/cpp/src/Thrift.h +++ b/lib/cpp/src/Thrift.h @@ -7,8 +7,14 @@ #ifndef _THRIFT_THRIFT_H_ #define _THRIFT_THRIFT_H_ 1 +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + #include +#ifdef HAVE_INTTYPES_H #include +#endif #include #include #include diff --git a/lib/cpp/src/protocol/TBinaryProtocol.cpp b/lib/cpp/src/protocol/TBinaryProtocol.cpp index 544ef885..5116e865 100644 --- a/lib/cpp/src/protocol/TBinaryProtocol.cpp +++ b/lib/cpp/src/protocol/TBinaryProtocol.cpp @@ -6,8 +6,50 @@ #include "TBinaryProtocol.h" +#include + using std::string; +// Use this to get around strict aliasing rules. +// For example, uint64_t i = bitwise_cast(returns_double()); +// The most obvious implementation is to just cast a pointer, +// but that doesn't work. +// For a pretty in-depth explanation of the problem, see +// http://www.cellperformance.com/mike_acton/2006/06/ (...) +// understanding_strict_aliasing.html +template +static inline To bitwise_cast(From from) { + BOOST_STATIC_ASSERT(sizeof(From) == sizeof(To)); + + // BAD!!! These are all broken with -O2. + //return *reinterpret_cast(&from); // BAD!!! + //return *static_cast(static_cast(&from)); // BAD!!! + //return *(To*)(void*)&from; // BAD!!! + + // Super clean and paritally blessed by section 3.9 of the standard. + //unsigned char c[sizeof(from)]; + //memcpy(c, &from, sizeof(from)); + //To to; + //memcpy(&to, c, sizeof(c)); + //return to; + + // Slightly more questionable. + // Same code emitted by GCC. + //To to; + //memcpy(&to, &from, sizeof(from)); + //return to; + + // Technically undefined, but almost universally supported, + // and the most efficient implementation. + union { + From f; + To t; + } u; + u.f = from; + return u.t; +} + + namespace facebook { namespace thrift { namespace protocol { uint32_t TBinaryProtocol::writeMessageBegin(const std::string& name, @@ -113,17 +155,12 @@ uint32_t TBinaryProtocol::writeI64(const int64_t i64) { } uint32_t TBinaryProtocol::writeDouble(const double dub) { - uint8_t b[8]; - uint8_t* d = (uint8_t*)&dub; - b[0] = d[7]; - b[1] = d[6]; - b[2] = d[5]; - b[3] = d[4]; - b[4] = d[3]; - b[5] = d[2]; - b[6] = d[1]; - b[7] = d[0]; - trans_->write((uint8_t*)b, 8); + BOOST_STATIC_ASSERT(sizeof(double) == sizeof(uint64_t)); + BOOST_STATIC_ASSERT(std::numeric_limits::is_iec559); + + uint64_t bits = bitwise_cast(dub); + bits = htonll(bits); + trans_->write((uint8_t*)&bits, 8); return 8; } @@ -291,18 +328,15 @@ uint32_t TBinaryProtocol::readI64(int64_t& i64) { } uint32_t TBinaryProtocol::readDouble(double& dub) { + BOOST_STATIC_ASSERT(sizeof(double) == sizeof(uint64_t)); + BOOST_STATIC_ASSERT(std::numeric_limits::is_iec559); + + uint64_t bits; uint8_t b[8]; - uint8_t d[8]; trans_->readAll(b, 8); - d[0] = b[7]; - d[1] = b[6]; - d[2] = b[5]; - d[3] = b[4]; - d[4] = b[3]; - d[5] = b[2]; - d[6] = b[1]; - d[7] = b[0]; - dub = *(double*)d; + bits = *(uint64_t*)b; + bits = ntohll(bits); + dub = bitwise_cast(bits); return 8; } diff --git a/lib/cpp/src/protocol/TProtocol.h b/lib/cpp/src/protocol/TProtocol.h index 6264bb6b..47d74f81 100644 --- a/lib/cpp/src/protocol/TProtocol.h +++ b/lib/cpp/src/protocol/TProtocol.h @@ -21,12 +21,24 @@ namespace facebook { namespace thrift { namespace protocol { using facebook::thrift::transport::TTransport; +#ifdef HAVE_ENDIAN_H +#include +#endif + #if __BYTE_ORDER == __BIG_ENDIAN -#define ntohll(n) (n) -#define htonll(n) (n) -#else -#define ntohll(n) ( (((unsigned long long)ntohl(n)) << 32) + ntohl(n >> 32) ) -#define htonll(n) ( (((unsigned long long)htonl(n)) << 32) + htonl(n >> 32) ) +# define ntohll(n) (n) +# define htonll(n) (n) +#elif __BYTE_ORDER == __LITTLE_ENDIAN +# if defined(__GNUC__) && defined(__GLIBC__) +# include +# define ntohll(n) bswap_64(n) +# define htonll(n) bswap_64(n) +# else /* GNUC & GLIBC */ +# define ntohll(n) ( (((unsigned long long)ntohl(n)) << 32) + ntohl(n >> 32) ) +# define htonll(n) ( (((unsigned long long)htonl(n)) << 32) + htonl(n >> 32) ) +# endif /* GNUC & GLIBC */ +#else /* __BYTE_ORDER */ +# error "Can't define htonll or ntohll!" #endif /** diff --git a/lib/cpp/src/transport/TFileTransport.cpp b/lib/cpp/src/transport/TFileTransport.cpp index aafdc211..84ea60d7 100644 --- a/lib/cpp/src/transport/TFileTransport.cpp +++ b/lib/cpp/src/transport/TFileTransport.cpp @@ -20,6 +20,9 @@ #include #include #include +#ifdef HAVE_STRINGS_H +#include +#endif #include #include -- 2.17.1