6
\$\begingroup\$

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 chars.

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ Seeing the API and its promises would certainly help; a sane one spares you from marshalling data manually. In any case, your code could be vulnerable to trap representation. \$\endgroup\$ – vnp Mar 4 '15 at 7:58
  • 1
    \$\begingroup\$ Thanks. I added the API reference. Marmalade IwNUI is not the most rigorous nor documented API by a long shot, but it seems to work passably so far. Trap representation would be somehow getting a garbled value in or out, being out of my expected range, or at worst a negative zero? So, I could add some pointer and range-checking sanity tests. \$\endgroup\$ – Dronz Mar 4 '15 at 8:31
  • 2
    \$\begingroup\$ As long as you are not porting the data written in one system to another, you should be fine. BTW, why didn't you just call the function with s3eSecureStoragePut(&variable, sizeof(uint32_t))? \$\endgroup\$ – glampert Mar 4 '15 at 18:30
  • 1
    \$\begingroup\$ @glampert In fact this will work on a machine with a different byte order. The least significant byte is always stored in the first byte of the stored buffer, and the first byte of the stored buffer is always copied into the least significant byte of the output, regardless of the endianness of the storing and loading machines. \$\endgroup\$ – ruds Mar 5 '15 at 20:19
  • 2
    \$\begingroup\$ Look up htonl and family for converting numbers into a standardized format for storage. \$\endgroup\$ – Martin York Mar 7 '15 at 2:07
6
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\begingroup\$ The changed signature is severely inconvenient, leading to cumbersome code. \$\endgroup\$ – Deduplicator Jul 4 '19 at 0:51
  • \$\begingroup\$ Why not store to unsigned char[] or std::uint8_t[] instead? If the signature can't be changed, we could cast the pointer within the function. \$\endgroup\$ – Toby Speight Jul 4 '19 at 8:20
1
\$\begingroup\$

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.

\$\endgroup\$
  • 4
    \$\begingroup\$ In C++ it is undefined behavior to write to a union through one member and read from another. \$\endgroup\$ – ruds Mar 6 '15 at 1:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.