Merge pull request #985 from leapmotion/config/unsigned-marshal

Fix marshaling of unsigned ints and 0
leapmotion · Aug 19, 2016 · b2775e8 · b2775e8
2 parents 52311f3 + 9585eca
commit b2775e8
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 15 deletions.
diff --git a/src/autowiring/marshaller.h b/src/autowiring/marshaller.h
@@ -5,8 +5,10 @@
 #include <cmath>
 #include <limits>
 #include <stdexcept>
+
 #include <string>
 #include <string.h>
+#include <stdlib.h>
 #include TYPE_TRAITS_HEADER
 
 namespace autowiring {
@@ -107,6 +109,9 @@ namespace autowiring {
     std::string marshal(const void* ptr) const override {
       std::string retVal;
       type val = *static_cast<const type*>(ptr);
+      if (val == 0)
+        return "0";
+
       bool pos = 0 < val;
       if (!pos)
         val *= ~0;
@@ -120,28 +125,20 @@ namespace autowiring {
         auto first = retVal.begin(), last = retVal.end();
         (first != last) && (first != --last);
         ++first
-      )
+        )
         std::swap(*first, *last);
       return retVal;
     }
 
     void unmarshal(void* ptr, const char* szValue) const override {
       type& value = *static_cast<type*>(ptr);
-      bool negative = *szValue == '-';
-      if(negative) {
-        // Skip over the sign, verify we aren't assigning to the wrong field type
-        szValue++;
-        if (std::is_unsigned<type>::value)
-          throw std::range_error("Attempted to set a signed value on an unsigned calibration field");
-      }
+      char* end = nullptr;
+      const auto llvalue = strtoll(szValue, &end, 10);
 
-      for (value = 0; *szValue; szValue++) {
-        if (*szValue < '0' || '9' < *szValue)
-          throw std::invalid_argument("String value is not an integer");
-        value = *szValue - '0' + value * 10;
-      }
-      if(negative)
-        value *= -1;
+      if (llvalue > std::numeric_limits<type>::max() || llvalue < std::numeric_limits<type>::min())
+        throw std::range_error("Overflow error, value is outside the range representable by this type.");
+
+      value = static_cast<type>(llvalue);
     }
 
     void copy(void* lhs, const void* rhs) const override {

diff --git a/src/autowiring/test/MarshallerTest.cpp b/src/autowiring/test/MarshallerTest.cpp
@@ -24,5 +24,28 @@ TEST_F(MarshallerTest, NegativeValues) {
   int y = 0xCDCDCDCD;
   std::string valY = b.marshal(&y);
   ASSERT_STREQ("-842150451", valY.c_str());
+
+  int z = 0;
+  std::string valZ = b.marshal(&z);
+  ASSERT_STREQ("0", valZ.c_str());
+}
+
+TEST_F(MarshallerTest, UnsignedValues) {
+  autowiring::marshaller<unsigned int> b;
+
+  int x = 0;
+  std::string valX = b.marshal(&x);
+  ASSERT_STREQ("0", valX.c_str());
 }
 
+TEST_F(MarshallerTest, RoundTripTest) {
+  autowiring::marshaller<unsigned int> b;
+
+  unsigned int x = 20;
+  std::string valX = b.marshal(&x);
+
+  unsigned int outX = 0;
+  b.unmarshal(&outX, valX.c_str());
+
+  ASSERT_EQ(outX, x);
+}