From d8f366daf2b4e0c088171a398f3168e1b83abfdc Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik@shafik.net>
Date: Thu, 18 Sep 2014 14:29:11 -0400
Subject: [PATCH] Fixing undefined behavior introduced by the incorrect use of
 comma operator with the conditional operator. The middle expression in a
 conditional operator between the ? and : is implicitly parenthesized but the
 end expression is not. Since the comma operator as he lowest precendence(
 http://en.cppreference.com/w/cpp/language/operator_precedence) this means the
 conditional operator will be evaluated first and then the expression on the
 right hand side of the comma operator will be evaluated. This leads to
 undefined behavior because the last member of the union being updated will
 not be the member that will be used next which is strictly undefined in C++
 although gcc and clang aloow this type punning as an extension but is clearly
 not portable behavior nor was this the intended behavior. instead of
 parenthesising the end expression I choose to use an if/else which is not
 subject to such easy to miss precdence issues. This Coliru live code
 demonstrates the bug with simple example:
 http://coliru.stacked-crooked.com/a/1041aaa8380feeaa the code also
 demonstrates the using the right warning flags gcc will generate a warning
 for this code.

---
 src/msgpack/type/fixint.hpp | 53 +++++++++++++++++++++++--
 src/msgpack/type/int.hpp    | 77 +++++++++++++++++++++++++++++++++----
 2 files changed, 119 insertions(+), 11 deletions(-)

diff --git a/src/msgpack/type/fixint.hpp b/src/msgpack/type/fixint.hpp
index ebe2696f..38633c66 100644
--- a/src/msgpack/type/fixint.hpp
+++ b/src/msgpack/type/fixint.hpp
@@ -115,16 +115,61 @@ inline packer<Stream>& operator<< (packer<Stream>& o, const type::fix_uint64& v)
 
 
 inline void operator<< (object& o, type::fix_int8 v)
-	{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
+	{ 
+	    if ( v.get() < 0 ) 
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v.get() ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v.get() ; 
+	    }
+	}
 
 inline void operator<< (object& o, type::fix_int16 v)
-	{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
+	{
+	    if ( v.get() < 0 )
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v.get() ;
+	    }
+            else
+	    { 
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v.get() ;
+	    }
+	}
 
 inline void operator<< (object& o, type::fix_int32 v)
-	{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
+	{
+	    if ( v.get() < 0 )
+	    {
+	        o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v.get() ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v.get() ;
+	    }
+	}
 
 inline void operator<< (object& o, type::fix_int64 v)
-	{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
+	{
+	    if( v.get() < 0 )
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v.get() ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v.get() ;
+	    }
+	}
+
 
 
 inline void operator<< (object& o, type::fix_uint8 v)
diff --git a/src/msgpack/type/int.hpp b/src/msgpack/type/int.hpp
index 6a1477ee..103fea7d 100644
--- a/src/msgpack/type/int.hpp
+++ b/src/msgpack/type/int.hpp
@@ -98,8 +98,16 @@ namespace detail {
 	template <>
 	struct object_char_sign<true> {
 		static inline void make(object& o, char v) {
-			v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v
-				: o.type = type::POSITIVE_INTEGER, o.via.u64 = v;
+			if( v < 0 )
+			{
+			    o.type = type::NEGATIVE_INTEGER ;
+			    o.via.i64 = v ;
+			}
+			else
+			{
+			    o.type = type::POSITIVE_INTEGER ;
+			    o.via.u64 = v ;
+			}
 		}
 	};
 
@@ -203,19 +211,74 @@ inline void operator<< (object& o, char v)
 
 
 inline void operator<< (object& o, signed char v)
-	{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
+	{
+	    if( v < 0 )
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v;
+	    }
+            else
+            {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v ;
+	    }
+	}
 
 inline void operator<< (object& o, signed short v)
-	{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
+	{
+	    if(v < 0 )
+	    {
+	        o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v ;
+	    }
+            else
+            {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v;
+	    }
+	}
 
 inline void operator<< (object& o, signed int v)
-	{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
+	{
+	    if( v < 0 )	
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v ;
+	    }
+	}
 
 inline void operator<< (object& o, signed long v)
-	{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
+	{
+	    if( v < 0 )
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v ;
+	    }
+	}
 
 inline void operator<< (object& o, signed long long v)
-	{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
+	{
+	    if( v < 0 )
+	    {
+		o.type = type::NEGATIVE_INTEGER ;
+		o.via.i64 = v ;
+	    }
+	    else
+	    {
+		o.type = type::POSITIVE_INTEGER ;
+		o.via.u64 = v ;
+	    }
+	}
 
 
 inline void operator<< (object& o, unsigned char v)