I need to pass some int32_t
values into and later retrieve them from a local storage API that uses void*
and size_t
parameters. Nothing else interprets the void*
values.
I hand-made some simple functions to translate back and forth between the types, which work, but I wonder if anyone would care to let me know if they see any problems, weaknesses or improvements I should make.
int32_t Char4ToInt(char* pChar4)
{
return (pChar4[3] << 24) | (pChar4[2] << 16) | (pChar4[1] << 8) | (pChar4[0]);
}
void StuffIntIntoChar4(char* pIntoChar4, int32_t val)
{
pIntoChar4[3] = val>>24;
pIntoChar4[2] = val>>16;
pIntoChar4[1] = val>>8;
pIntoChar4[0] = val;
}
If they were for more general use, I'd probably check the char*
parameters for errors, but I only use them where they are passed
char valBuf[4];
as valBuf
, so I know it always points to 4 allocated char
s.
The API I am doing this for is s3eSecureStoragePut and s3eSecureStorageGet from the Marmalade API - the documentation doesn't go into great detail about guarantees, but it seems to work to store and retrieve a blob of bytes of the supplied size. This is a smartphone App and I just save my blob before exit and load it on entry, copying the data over if there is no error, otherwise using default data. The data just goes into UI controls (in this case, slider position values). However if there are weaknesses in using these functions more broadly, I'd be interested in any suggestions, as I might end up using them (or whatever gets pointed out as better) for some later project if they seem reasonable.
2 Answers 2
This is almost right, but it has two flaws:
First, you can improve type safety by passing char(&)[4] instead of char*:
int32_t Char4ToInt(char (&pChar4)[4]);
void StuffIntIntoChar4(char (&pIntoChar4)[4], int32_t val);
Second, you are running into undefined behavior. In the C++11 standard (section [expr.shift]), it says
The value of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are zero-filled. If E1 has an unsigned type, the value of the result is \$E1 ×ばつ 2^{E2}\,ドル reduced modulo one more than the maximum value representable in the result type. Otherwise, if E1 has a signed type and non-negative value, and \$E1 ×ばつ 2^{E2}\$ is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.
The last part is what you're running into -- you're left-shifting a negative value in some cases, which is undefined behavior.
You have a couple of choices, depending on your portability requirements.
If you will always run this on the same architecture (or in any case only on architectures with the same byte ordering), you can store like this:
void SerializeInt32(char (&buf)[4], int32_t val)
{
std::memcpy(buf, &val, 4);
}
int32_t ParseInt32(const char (&buf)[4])
{
int32_t val;
std::memcpy(&val, buf, 4);
return val;
}
If you would like instead to be able to store with a program built on one platform and load on a program built on a different platform (as long as its implementation-defined unsigned int -> int conversion uses two's complement, which is almost all modern platforms), you can write it like this:
void SerializeInt32(char (&buf)[4], int32_t val)
{
uint32_t uval = val;
buf[0] = uval;
buf[1] = uval >> 8;
buf[2] = uval >> 16;
buf[3] = uval >> 24;
}
int32_t ParseInt32(const char (&buf)[4])
{
// This prevents buf[i] from being promoted to a signed int.
uint32_t u0 = buf[0], u1 = buf[1], u2 = buf[2], u3 = buf[3];
uint32_t uval = u0 | (u1 << 8) | (u2 << 16) | (u3 << 24);
return uval;
}
-
\$\begingroup\$ The changed signature is severely inconvenient, leading to cumbersome code. \$\endgroup\$Deduplicator– Deduplicator2019年07月04日 00:51:02 +00:00Commented Jul 4, 2019 at 0:51
-
\$\begingroup\$ Why not store to
unsigned char[]
orstd::uint8_t[]
instead? If the signature can't be changed, we could cast the pointer within the function. \$\endgroup\$Toby Speight– Toby Speight2019年07月04日 08:20:50 +00:00Commented Jul 4, 2019 at 8:20
For readability and simplicity, why don't we use a union
?
union converter {
char c[4];
int32_t i;
}
Now, to convert, it's as simple as this:
union converter conv;
conv.i = yourInt32Value;
char *cString = conv.c;
or
union converter conv;
conv.c = yourCStringValue;
int32_t i = conv.i;
Remember to pay attention to your endianness, however.
union converter {
char c[4];
int32_t i;
};
int main(int argc, const char * argv[]) {
union converter conv;
conv.c[0] = 0xFF;
conv.c[1] = 0xEE;
conv.c[2] = 0xDD;
conv.c[3] = 0xCC;
std::cout << std::hex << conv.i << std::endl;
return 0;
}
The above snippet prints: ccddeeff
(on my machine).
Consider taking a look at this StackOverflow answer for dealing with compile-time and run-time endianness checking.
-
5\$\begingroup\$ In C++ it is undefined behavior to write to a union through one member and read from another. \$\endgroup\$ruds– ruds2015年03月06日 01:43:03 +00:00Commented Mar 6, 2015 at 1:43
s3eSecureStoragePut(&variable, sizeof(uint32_t))
? \$\endgroup\$