Read unsigned integers in WKBReader

OGC WKB spec specifies that the integers are unsigned. Use of signed integers is probably a carryover from Java, which does not have unsigned integers. Clear type conversion warnings in WKBReader and use C++11 int64_t type.
libgeos · Jan 29, 2021 · 47bc279 · 47bc279
1 parent 6ceec43
commit 47bc279
Show file tree

Hide file tree

Showing 12 changed files with 169 additions and 87 deletions.
diff --git a/benchmarks/ClassSizes.cpp b/benchmarks/ClassSizes.cpp
@@ -76,6 +76,6 @@ main()
     check(triangulate::quadedge::QuadEdge);
     check(triangulate::quadedge::QuadEdgeQuartet);
     check(triangulate::quadedge::Vertex);
-    check(int64);
+    check(int64_t);
 }
 
diff --git a/include/geos/constants.h b/include/geos/constants.h
@@ -31,9 +31,6 @@ typedef __int64 int64;
 #include <limits>
 #include <cinttypes>
 
-
-typedef int64_t int64;
-
 namespace geos {
 
 constexpr double MATH_PI = 3.14159265358979323846;

diff --git a/include/geos/io/ByteOrderDataInStream.h b/include/geos/io/ByteOrderDataInStream.h
@@ -21,6 +21,7 @@
 #define GEOS_IO_BYTEORDERDATAINSTREAM_H
 
 #include <geos/export.h>
+#include <cstdint>
 
 //#include <geos/io/ParseException.h>
 //#include <geos/io/ByteOrderValues.h>
@@ -50,9 +51,11 @@ class GEOS_DLL ByteOrderDataInStream {
 
     unsigned char readByte(); // throws ParseException
 
-    int readInt(); // throws ParseException
+    int32_t readInt(); // throws ParseException
 
-    long readLong(); // throws ParseException
+    uint32_t readUnsigned(); // throws ParseException
+
+    int64_t readLong(); // throws ParseException
 
     double readDouble(); // throws ParseException
 

diff --git a/include/geos/io/ByteOrderDataInStream.inl b/include/geos/io/ByteOrderDataInStream.inl
@@ -61,7 +61,7 @@ ByteOrderDataInStream::readByte() // throws ParseException
     return ret;
 }
 
-INLINE int
+INLINE int32_t
 ByteOrderDataInStream::readInt()
 {
     if(size() < 4) {
@@ -72,6 +72,17 @@ ByteOrderDataInStream::readInt()
     return ret;
 }
 
+INLINE uint32_t
+ByteOrderDataInStream::readUnsigned()
+{
+    if(size() < 4) {
+        throw ParseException("Unexpected EOF parsing WKB");
+    }
+    auto ret =  ByteOrderValues::getUnsigned(buf , byteOrder);
+    buf += 4;
+    return ret;
+}
+
 INLINE long
 ByteOrderDataInStream::readLong()
 {

diff --git a/include/geos/io/ByteOrderValues.h b/include/geos/io/ByteOrderValues.h
@@ -21,7 +21,7 @@
 #define GEOS_IO_BYTEORDERVALUES_H
 
 #include <geos/export.h>
-#include <geos/constants.h>
+#include <cstdint>
 
 namespace geos {
 namespace io {
@@ -43,11 +43,14 @@ class GEOS_DLL ByteOrderValues {
         ENDIAN_LITTLE = 1
     };
 
-    static int getInt(const unsigned char* buf, int byteOrder);
-    static void putInt(int intValue, unsigned char* buf, int byteOrder);
+    static int32_t getInt(const unsigned char* buf, int byteOrder);
+    static void putInt(int32_t intValue, unsigned char* buf, int byteOrder);
 
-    static int64 getLong(const unsigned char* buf, int byteOrder);
-    static void putLong(int64 longValue, unsigned char* buf, int byteOrder);
+    static uint32_t getUnsigned(const unsigned char* buf, int byteOrder);
+    static void putUnsigned(uint32_t intValue, unsigned char* buf, int byteOrder);
+
+    static int64_t getLong(const unsigned char* buf, int byteOrder);
+    static void putLong(int64_t longValue, unsigned char* buf, int byteOrder);
 
     static double getDouble(const unsigned char* buf, int byteOrder);
     static void putDouble(double doubleValue, unsigned char* buf, int byteOrder);

diff --git a/include/geos/io/WKBReader.h b/include/geos/io/WKBReader.h
@@ -156,7 +156,7 @@ class GEOS_DLL WKBReader {
 
     std::unique_ptr<geom::GeometryCollection> readGeometryCollection();
 
-    std::unique_ptr<geom::CoordinateSequence> readCoordinateSequence(int); // throws IOException
+    std::unique_ptr<geom::CoordinateSequence> readCoordinateSequence(unsigned int); // throws IOException
 
     void readCoordinate(); // throws IOException
 

diff --git a/include/geos/precision/CommonBits.h b/include/geos/precision/CommonBits.h
@@ -16,7 +16,7 @@
 #define GEOS_PRECISION_COMMONBITS_H
 
 #include <geos/export.h>
-#include <geos/constants.h> // for int64
+#include <cstdint>
 
 namespace geos {
 namespace precision { // geos.precision
@@ -38,9 +38,9 @@ class GEOS_DLL CommonBits {
 
     int commonMantissaBitsCount;
 
-    int64 commonBits;
+    int64_t commonBits;
 
-    int64 commonSignExp;
+    int64_t commonSignExp;
 
 public:
 
@@ -51,7 +51,7 @@ class GEOS_DLL CommonBits {
      * @param num
      * @return the bit pattern for the sign and exponent
      */
-    static int64 signExpBits(int64 num);
+    static int64_t signExpBits(int64_t num);
 
     /** \brief
      * This computes the number of common most-significant
@@ -65,7 +65,7 @@ class GEOS_DLL CommonBits {
      * @param num2
      * @return the number of common most-significant mantissa bits
      */
-    static int numCommonMostSigMantissaBits(int64 num1, int64 num2);
+    static int numCommonMostSigMantissaBits(int64_t num1, int64_t num2);
 
     /** \brief
      * Zeroes the lower n bits of a bitstring.
@@ -74,7 +74,7 @@ class GEOS_DLL CommonBits {
      * @param nBits the number of bits to zero
      * @return the zeroed bitstring
      */
-    static int64 zeroLowerBits(int64 bits, int nBits);
+    static int64_t zeroLowerBits(int64_t bits, int nBits);
 
     /** \brief
      * Extracts the i'th bit of a bitstring.
@@ -83,7 +83,7 @@ class GEOS_DLL CommonBits {
      * @param i the bit to extract
      * @return the value of the extracted bit
      */
-    static int getBit(int64 bits, int i);
+    static int getBit(int64_t bits, int i);
 
     CommonBits();
 

diff --git a/src/io/ByteOrderValues.cpp b/src/io/ByteOrderValues.cpp
@@ -31,27 +31,65 @@
 namespace geos {
 namespace io { // geos.io
 
-int
+int32_t
 ByteOrderValues::getInt(const unsigned char* buf, int byteOrder)
 {
     if(byteOrder == ENDIAN_BIG) {
-        return ((int)(buf[0] & 0xff) << 24) |
-               ((int)(buf[1] & 0xff) << 16) |
-               ((int)(buf[2] & 0xff) << 8) |
-               ((int)(buf[3] & 0xff));
+        return ((int32_t)(buf[0] & 0xff) << 24) |
+               ((int32_t)(buf[1] & 0xff) << 16) |
+               ((int32_t)(buf[2] & 0xff) << 8) |
+               ((int32_t)(buf[3] & 0xff));
     }
     else { // ENDIAN_LITTLE
         assert(byteOrder == ENDIAN_LITTLE);
 
-        return ((int)(buf[3] & 0xff) << 24) |
-               ((int)(buf[2] & 0xff) << 16) |
-               ((int)(buf[1] & 0xff) << 8) |
-               ((int)(buf[0] & 0xff));
+        return ((int32_t)(buf[3] & 0xff) << 24) |
+               ((int32_t)(buf[2] & 0xff) << 16) |
+               ((int32_t)(buf[1] & 0xff) << 8) |
+               ((int32_t)(buf[0] & 0xff));
+    }
+}
+
+uint32_t
+ByteOrderValues::getUnsigned(const unsigned char* buf, int byteOrder)
+{
+    if(byteOrder == ENDIAN_BIG) {
+        return ((uint32_t)(buf[0] & 0xff) << 24) |
+               ((uint32_t)(buf[1] & 0xff) << 16) |
+               ((uint32_t)(buf[2] & 0xff) << 8) |
+               ((uint32_t)(buf[3] & 0xff));
+    }
+    else { // ENDIAN_LITTLE
+        assert(byteOrder == ENDIAN_LITTLE);
+
+        return ((uint32_t)(buf[3] & 0xff) << 24) |
+               ((uint32_t)(buf[2] & 0xff) << 16) |
+               ((uint32_t)(buf[1] & 0xff) << 8) |
+               ((uint32_t)(buf[0] & 0xff));
+    }
+}
+
+void
+ByteOrderValues::putInt(int32_t intValue, unsigned char* buf, int byteOrder)
+{
+    if(byteOrder == ENDIAN_BIG) {
+        buf[0] = (unsigned char)(intValue >> 24);
+        buf[1] = (unsigned char)(intValue >> 16);
+        buf[2] = (unsigned char)(intValue >> 8);
+        buf[3] = (unsigned char) intValue;
+    }
+    else { // ENDIAN_LITTLE
+        assert(byteOrder == ENDIAN_LITTLE);
+
+        buf[3] = (unsigned char)(intValue >> 24);
+        buf[2] = (unsigned char)(intValue >> 16);
+        buf[1] = (unsigned char)(intValue >> 8);
+        buf[0] = (unsigned char) intValue;
     }
 }
 
 void
-ByteOrderValues::putInt(int intValue, unsigned char* buf, int byteOrder)
+ByteOrderValues::putUnsigned(uint32_t intValue, unsigned char* buf, int byteOrder)
 {
     if(byteOrder == ENDIAN_BIG) {
         buf[0] = (unsigned char)(intValue >> 24);
@@ -69,37 +107,37 @@ ByteOrderValues::putInt(int intValue, unsigned char* buf, int byteOrder)
     }
 }
 
-int64
+int64_t
 ByteOrderValues::getLong(const unsigned char* buf, int byteOrder)
 {
     if(byteOrder == ENDIAN_BIG) {
         return
-            (int64)(buf[0]) << 56
-            | (int64)(buf[1] & 0xff) << 48
-            | (int64)(buf[2] & 0xff) << 40
-            | (int64)(buf[3] & 0xff) << 32
-            | (int64)(buf[4] & 0xff) << 24
-            | (int64)(buf[5] & 0xff) << 16
-            | (int64)(buf[6] & 0xff) <<  8
-            | (int64)(buf[7] & 0xff);
+            (int64_t)(buf[0]) << 56
+            | (int64_t)(buf[1] & 0xff) << 48
+            | (int64_t)(buf[2] & 0xff) << 40
+            | (int64_t)(buf[3] & 0xff) << 32
+            | (int64_t)(buf[4] & 0xff) << 24
+            | (int64_t)(buf[5] & 0xff) << 16
+            | (int64_t)(buf[6] & 0xff) <<  8
+            | (int64_t)(buf[7] & 0xff);
     }
     else { // ENDIAN_LITTLE
         assert(byteOrder == ENDIAN_LITTLE);
 
         return
-            (int64)(buf[7]) << 56
-            | (int64)(buf[6] & 0xff) << 48
-            | (int64)(buf[5] & 0xff) << 40
-            | (int64)(buf[4] & 0xff) << 32
-            | (int64)(buf[3] & 0xff) << 24
-            | (int64)(buf[2] & 0xff) << 16
-            | (int64)(buf[1] & 0xff) <<  8
-            | (int64)(buf[0] & 0xff);
+            (int64_t)(buf[7]) << 56
+            | (int64_t)(buf[6] & 0xff) << 48
+            | (int64_t)(buf[5] & 0xff) << 40
+            | (int64_t)(buf[4] & 0xff) << 32
+            | (int64_t)(buf[3] & 0xff) << 24
+            | (int64_t)(buf[2] & 0xff) << 16
+            | (int64_t)(buf[1] & 0xff) <<  8
+            | (int64_t)(buf[0] & 0xff);
     }
 }
 
 void
-ByteOrderValues::putLong(int64 longValue, unsigned char* buf, int byteOrder)
+ByteOrderValues::putLong(int64_t longValue, unsigned char* buf, int byteOrder)
 {
     if(byteOrder == ENDIAN_BIG) {
         buf[0] = (unsigned char)(longValue >> 56);
@@ -128,7 +166,7 @@ ByteOrderValues::putLong(int64 longValue, unsigned char* buf, int byteOrder)
 double
 ByteOrderValues::getDouble(const unsigned char* buf, int byteOrder)
 {
-    int64 longValue = getLong(buf, byteOrder);
+    int64_t longValue = getLong(buf, byteOrder);
     double ret;
     std::memcpy(&ret, &longValue, sizeof(double));
     return ret;
@@ -137,7 +175,7 @@ ByteOrderValues::getDouble(const unsigned char* buf, int byteOrder)
 void
 ByteOrderValues::putDouble(double doubleValue, unsigned char* buf, int byteOrder)
 {
-    int64 longValue;
+    int64_t longValue;
     std::memcpy(&longValue, &doubleValue, sizeof(double));
 #if DEBUG_BYTEORDER_VALUES
     std::cout << "ByteOrderValues::putDouble(" << doubleValue <<