Skip to content

Commit 30e6b6d

Browse files
committed
Write UUID transformation more portably
The previous version did not handle endianness correctly. MSVC refused to compile the code that assumed we knew what we were doing, which turned out to be a good thing.
1 parent b48a0e9 commit 30e6b6d

File tree

1 file changed

+42
-11
lines changed

1 file changed

+42
-11
lines changed

src/gromacs/hardware/tests/device_management.cpp

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444

4545
#include "config.h"
4646

47+
#include <cstdint>
48+
4749
#include <algorithm>
4850
#include <memory>
4951
#include <string>
@@ -106,36 +108,65 @@ TEST(DevicesManagerTest, Serialization)
106108
}
107109
}
108110

111+
template<std::size_t N>
112+
uint32_t uint32FromBytes(const std::array<std::byte, N>& data, const std::size_t byteOffset)
113+
{
114+
if (byteOffset + sizeof(uint32_t) > N)
115+
{
116+
throw std::invalid_argument("byteOffset would read out of bounds");
117+
}
118+
119+
// Integer to copy the bytes to
120+
uint32_t result;
121+
122+
// Copy the bytes, assuming little-endian layout. The compiler
123+
// generally elides this away.
124+
std::memcpy(&result, &(data[byteOffset]), sizeof(result));
125+
if (GMX_INTEGER_BIG_ENDIAN)
126+
{
127+
// Change the endianness to match the hardware
128+
const auto* ptr = reinterpret_cast<const uint8_t*>(&result);
129+
return (ptr[0] << 24) | (ptr[1] << 16) | (ptr[2] << 8) | ptr[3];
130+
}
131+
else
132+
{
133+
return result;
134+
}
135+
}
136+
109137
std::string uuidToString(const std::array<std::byte, 16>& uuid)
110138
{
111139
// Write a string in the frequently used 8-4-4-4-12 format,
112140
// xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, where every x represents 4 bits
113141
std::string result;
114142
result.reserve(37);
115-
const auto* uuidIt = uuid.cbegin();
116-
for (int i = 0; i < 4; ++i, ++uuidIt)
143+
144+
// Compilers tend to think uuid.size() is not a constant expression!
145+
constexpr std::size_t uuidWidthInBytes = 16;
146+
std::size_t byteOffset = 0;
147+
for (int i = 0; i < 4; ++i, byteOffset += sizeof(uint32_t))
117148
{
118-
result += gmx::formatString("%2.2x", static_cast<unsigned int>(*uuidIt));
149+
result += gmx::formatString("%2.2x", uint32FromBytes<uuidWidthInBytes>(uuid, byteOffset));
119150
}
120151
result.append(1, '-');
121-
for (int i = 0; i < 2; ++i, ++uuidIt)
152+
for (int i = 0; i < 2; ++i, byteOffset += sizeof(uint32_t))
122153
{
123-
result += gmx::formatString("%2.2x", static_cast<unsigned int>(*uuidIt));
154+
result += gmx::formatString("%2.2x", uint32FromBytes<uuidWidthInBytes>(uuid, byteOffset));
124155
}
125156
result.append(1, '-');
126-
for (int i = 0; i < 2; ++i, ++uuidIt)
157+
for (int i = 0; i < 2; ++i, byteOffset += sizeof(uint32_t))
127158
{
128-
result += gmx::formatString("%2.2x", static_cast<unsigned int>(*uuidIt));
159+
result += gmx::formatString("%2.2x", uint32FromBytes<uuidWidthInBytes>(uuid, byteOffset));
129160
}
130161
result.append(1, '-');
131-
for (int i = 0; i < 2; ++i, ++uuidIt)
162+
for (int i = 0; i < 2; ++i, byteOffset += sizeof(uint32_t))
132163
{
133-
result += gmx::formatString("%2.2x", static_cast<unsigned int>(*uuidIt));
164+
result += gmx::formatString("%2.2x", uint32FromBytes<uuidWidthInBytes>(uuid, byteOffset));
134165
}
135166
result.append(1, '-');
136-
for (int i = 0; i < 6; ++i, ++uuidIt)
167+
for (int i = 0; i < 6; ++i, byteOffset += sizeof(uint32_t))
137168
{
138-
result += gmx::formatString("%2.2x", static_cast<unsigned int>(*uuidIt));
169+
result += gmx::formatString("%2.2x", uint32FromBytes<uuidWidthInBytes>(uuid, byteOffset));
139170
}
140171
return result;
141172
}

0 commit comments

Comments
 (0)