fix swap functions correctness (#3)

Author: anonrig

include/nbytes.h

@@ -93,9 +93,9 @@ void ForceAscii(const char *src, char *dst, size_t len);
 
 // Swaps bytes in place. nbytes is the number of bytes to swap and must be a
 // multiple of the word size (checked by function).
-bool SwapBytes16(void *data, size_t nbytes);
-bool SwapBytes32(void *data, size_t nbytes);
-bool SwapBytes64(void *data, size_t nbytes);
+bool SwapBytes16(char *data, size_t nbytes);
+bool SwapBytes32(char *data, size_t nbytes);
+bool SwapBytes64(char *data, size_t nbytes);
 
 // ============================================================================
 // Base64 (legacy)

src/nbytes.cpp

@@ -43,7 +43,7 @@ namespace {
 #endif
 }  // namespace
 
-bool SwapBytes16(void *data, size_t nbytes) {
+bool SwapBytes16(char *data, size_t nbytes) {
   if (nbytes % sizeof(uint16_t) != 0) return false;
 
 #if defined(_MSC_VER)
@@ -59,17 +59,16 @@ bool SwapBytes16(void *data, size_t nbytes) {
 #endif
 
   uint16_t temp;
-  uint8_t *ptr = reinterpret_cast<uint8_t *>(data);
-  for (size_t i = 0; i < nbytes; i += sizeof(uint16_t)) {
-    memcpy(&temp, &ptr[i], sizeof(uint16_t));
+  for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
+    memcpy(&temp, &data[i], sizeof(temp));
     temp = BSWAP_2(temp);
-    memcpy(&ptr[i], &temp, sizeof(uint16_t));
+    memcpy(&data[i], &temp, sizeof(temp));
   }
 
   return true;
 }
 
-bool SwapBytes32(void *data, size_t nbytes) {
+bool SwapBytes32(char *data, size_t nbytes) {
   if (nbytes % sizeof(uint32_t) != 0) return false;
 
 #if defined(_MSC_VER)
@@ -84,18 +83,17 @@ bool SwapBytes32(void *data, size_t nbytes) {
   }
 #endif
 
-  uint32_t temp = 0;
-  uint8_t *ptr = reinterpret_cast<uint8_t *>(data);
-  for (size_t i = 0; i < nbytes; i += sizeof(uint32_t)) {
-    memcpy(&temp, &ptr[i], sizeof(uint32_t));
+  uint32_t temp;
+  for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
+    memcpy(&temp, &data[i], sizeof(temp));
     temp = BSWAP_4(temp);
-    memcpy(&ptr[i], &temp, sizeof(uint32_t));
+    memcpy(&data[i], &temp, sizeof(temp));
   }
 
   return true;
 }
 
-bool SwapBytes64(void *data, size_t nbytes) {
+bool SwapBytes64(char *data, size_t nbytes) {
   if (nbytes % sizeof(uint64_t) != 0) return false;
 
 #if defined(_MSC_VER)
@@ -110,12 +108,11 @@ bool SwapBytes64(void *data, size_t nbytes) {
   }
 #endif
 
-  uint64_t temp = 0;
-  uint8_t *ptr = reinterpret_cast<uint8_t *>(data);
-  for (size_t i = 0; i < nbytes; i += sizeof(uint64_t)) {
-    memcpy(&temp, &ptr[i], sizeof(uint64_t));
+  uint64_t temp;
+  for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
+    memcpy(&temp, &data[i], sizeof(temp));
     temp = BSWAP_8(temp);
-    memcpy(&ptr[i], &temp, sizeof(uint64_t));
+    memcpy(&data[i], &temp, sizeof(temp));
   }
 
   return true;

tests/basic.cpp

@@ -4,4 +4,26 @@
 
 #include <gtest/gtest.h>
 
-TEST(basic, it_works) { SUCCEED(); }
+TEST(basic, swap_bytes16) {
+  std::vector<char> input = {1, 2, 3, 4, 5, 6, 7, 8};
+  nbytes::SwapBytes16(input.data(), input.size());
+  std::vector<char> expected = {2, 1, 4, 3, 6, 5, 8, 7};
+  EXPECT_EQ(input, expected);
+  SUCCEED();
+}
+
+TEST(basic, swap_bytes32) {
+  std::vector<char> input = {1, 2, 3, 4, 5, 6, 7, 8};
+  nbytes::SwapBytes32(input.data(), input.size());
+  std::vector<char> expected = {4, 3, 2, 1, 8, 7, 6, 5};
+  EXPECT_EQ(input, expected);
+  SUCCEED();
+}
+
+TEST(basic, swap_bytes64) {
+  std::vector<char> input = {1, 2, 3, 4, 5, 6, 7, 8};
+  nbytes::SwapBytes64(input.data(), input.size());
+  std::vector<char> expected = {8, 7, 6, 5, 4, 3, 2, 1};
+  EXPECT_EQ(input, expected);
+  SUCCEED();
+}