[ 
https://issues.apache.org/jira/browse/THRIFT-1666?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Brandon Streiff updated THRIFT-1666:
------------------------------------

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

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

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:
{code}#  define htonll(n) ( (((uint64_t)htonl(n)) << 32) + htonl(n >> 32) 
){code}

[htonl() takes a u_long 
parameter|http://msdn.microsoft.com/en-us/library/windows/desktop/ms738556%28v=vs.85%29.aspx],
 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()|http://msdn.microsoft.com/en-us/library/a3140177%28v=vs.100%29.aspx]
 function in the MSVC CRT that we can use without all of the shifting that 
calling ntohl twice requires.

Patch follows:
{code}
--- 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

 /**
{code}

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

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

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:
{code}#  define htonll(n) ( (((uint64_t)htonl(n)) << 32) + htonl(n >> 32) 
){code}

[ntohl() takes a u_long 
parameter|http://msdn.microsoft.com/en-us/library/windows/desktop/ms740069%28v=vs.85%29.aspx],
 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()|http://msdn.microsoft.com/en-us/library/a3140177%28v=vs.100%29.aspx]
 function in the MSVC CRT that we can use without all of the shifting that 
calling ntohl twice requires.

Patch follows:
{code}
--- 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

 /**
{code}


(Whoops, bug description noted ntohl instead of htonl... though it would have 
the same problem. Fixed for clarity.)
                
> htonll usage in TBinaryProtocol.tcc generates warning with MSVC2010
> -------------------------------------------------------------------
>
>                 Key: THRIFT-1666
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1666
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>    Affects Versions: 0.8
>         Environment: Microsoft Visual Studio 10.0 (2010).
>            Reporter: Brandon Streiff
>            Priority: Trivial
>              Labels: msvc, warning
>
> When using the Thrift library I get warnings like the following:
> {code}thrift\protocol\TBinaryProtocol.tcc(161): warning C4244: 'argument' : 
> conversion from 'const int64_t' to 'u_long', possible loss of data{code}
> 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:
> {code}#  define htonll(n) ( (((uint64_t)htonl(n)) << 32) + htonl(n >> 32) 
> ){code}
> [htonl() takes a u_long 
> parameter|http://msdn.microsoft.com/en-us/library/windows/desktop/ms738556%28v=vs.85%29.aspx],
>  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()|http://msdn.microsoft.com/en-us/library/a3140177%28v=vs.100%29.aspx]
>  function in the MSVC CRT that we can use without all of the shifting that 
> calling ntohl twice requires.
> Patch follows:
> {code}
> --- 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
>  /**
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to