Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-1666

htonll usage in TBinaryProtocol.tcc generates warning with MSVC2010

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 0.8
    • Fix Version/s: 0.9.1
    • Component/s: C++ - Library
    • Labels:
    • Environment:

      Microsoft Visual Studio 10.0 (2010).

    • Patch Info:
      Patch Available

      Description

      When using the Thrift library I get warnings like the following:

      thrift\protocol\TBinaryProtocol.tcc(161): warning C4244: 'argument' : conversion from 'const int64_t' to 'u_long', possible loss of data

      This is because TBinaryProtocol.tcc uses htonll on an incoming const int64_t parameter to perform byteswapping, and in TProtocol.h htonll gets defined like so:

      #  define htonll(n) ( (((uint64_t)htonl(n)) << 32) + htonl(n >> 32) )

      htonl() takes a u_long parameter, so MSVC warns when passing in a uint64_t parameter because it truncates. This could have been fixed by casting the input parameters, but there's also a nice handy _byteswap_uint64() function in the MSVC CRT that we can use without all of the shifting that calling ntohl twice requires.

      Patch follows:

      --- a\lib\cpp\src\protocol\TProtocol.h 31 Jul 2012  9:18:02 AM
      +++ b\lib\cpp\src\protocol\TProtocol.h 31 Jul 2012  9:19:48 AM
      @@ -127,140 +127,143 @@
       #  define letohll(n) (n)
       # if defined(__GNUC__) && defined(__GLIBC__)
       #  include <byteswap.h>
       #  define ntohll(n) bswap_64(n)
       #  define htonll(n) bswap_64(n)
      -# else /* GNUC & GLIBC */
      +# elif defined(_MSC_VER) /* Microsoft Visual C++ */
      +#  define ntohll(n) ( _byteswap_uint64((uint64_t)n) )
      +#  define htonll(n) ( _byteswap_uint64((uint64_t)n) )
      +# else /* Not GNUC/GLIBC or MSVC */
       #  define ntohll(n) ( (((uint64_t)ntohl(n)) << 32) + ntohl(n >> 32) )
       #  define htonll(n) ( (((uint64_t)htonl(n)) << 32) + htonl(n >> 32) )
      -# endif /* GNUC & GLIBC */
      +# endif /* GNUC/GLIBC or MSVC or something else */
       #else /* __BYTE_ORDER */
       # error "Can't define htonll or ntohll!"
       #endif
      
       /**
      

        Attachments

          Activity

            People

            • Assignee:
              jfarrell Jake Farrell
              Reporter:
              bstreiff Brandon Streiff

              Dates

              • Created:
                Updated:
                Resolved:

                Issue deployment