C/C++ packing signed char into int

Question

I have need to pack four signed bytes into 32-bit integral type. this is what I came up to:

int32_t byte(int8_t c) { return (unsigned char)c; }

int pack(char c0, char c1, ...) {
  return byte(c0) | byte(c1) << 8 | ...;
}

is this a good solution? Is it portable (not in communication sense)? is there a ready-made solution, perhaps boost?

issue I am mostly concerned about is bit order when converting of negative bits from char to int. I do not know what the correct behavior should be.

Thanks

Answer 1

"Goodness"
IMHO, this is the best solution you're going to get for this. EDIT: though I'd use static_cast<unsigned int> instead of the C-style cast, and I'd probably not use a separate method to hide the cast....

Portability:
There is going to be no portable way to do this because nothing says char has to be eight bits, and nothing says unsigned int needs to be 4 bytes wide.

Furthermore, you're relying on endianness and therefore data pack'd on one architecture will not be usable on one with the opposite endianness.

is there a ready-made solution, perhaps boost?
Not of which I am aware.

Answer 2

char isn't guaranteed to be signed or unsigned (on PowerPC Linux, char defaults to unsigned). Spread the word!

What you want is something like this macro:

#include <stdint.h> /* Needed for uint32_t and uint8_t */

#define PACK(c0, c1, c2, c3) \
    (((uint32_t)(uint8_t)(c0) << 24) | \
    ((uint32_t)(uint8_t)(c1) << 16) | \
    ((uint32_t)(uint8_t)(c2) << 8) | \
    ((uint32_t)(uint8_t)(c3)))

It's ugly mainly because it doesn't play well with C's order of operations. Also, the backslash-returns are there so this macro doesn't have to be one big long line.

Also, the reason we cast to uint8_t before casting to uint32_t is to prevent unwanted sign extension.

Answer 3

This is based on Grant Peters and Joey Adams' answers, extended to show how to unpack the signed values (the unpack functions rely on the modulo rules of unsigned values in C):

(As Steve Jessop noted in comments, there is no need for separate pack_s and pack_u functions).

inline uint32_t pack(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3)
{
    return ((uint32_t)c0 << 24) | ((uint32_t)c1 << 16) |
        ((uint32_t)c2 << 8) | (uint32_t)c3;
}

inline uint8_t unpack_c3_u(uint32_t p)
{
    return p >> 24;
}

inline uint8_t unpack_c2_u(uint32_t p)
{
    return p >> 16;
}

inline uint8_t unpack_c1_u(uint32_t p)
{
    return p >> 8;
}

inline uint8_t unpack_c0_u(uint32_t p)
{
    return p;
}

inline uint8_t unpack_c3_s(uint32_t p)
{
    int t = unpack_c3_u(p);
    return t <= 127 ? t : t - 256;
}

inline uint8_t unpack_c2_s(uint32_t p)
{
    int t = unpack_c2_u(p);
    return t <= 127 ? t : t - 256;
}

inline uint8_t unpack_c1_s(uint32_t p)
{
    int t = unpack_c1_u(p);
    return t <= 127 ? t : t - 256;
}

inline uint8_t unpack_c0_s(uint32_t p)
{
    int t = unpack_c0_u(p);
    return t <= 127 ? t : t - 256;
}

(These are necessary rather than simply casting back to int8_t, because the latter may cause an implementation-defined signal to be raised if the value is over 127, so it's not strictly portable).

Answer 4

You can avoid casts with implicit conversions:

uint32_t pack_helper(uint32_t c0, uint32_t c1, uint32_t c2, uint32_t c3) {
    return c0 | (c1 << 8) | (c2 << 16) | (c3 << 24);
}

uint32_t pack(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) {
    return pack_helper(c0, c1, c2, c3);
}

The idea is that you see "convert all the parameters correctly. Shift and combine them", rather than "for each parameter, convert it correctly, shift and combine it". Not much in it, though.

Then:

template <int N>
uint8_t unpack_u(uint32_t packed) {
    // cast to avoid potential warnings for implicit narrowing conversion
    return static_cast<uint8_t>(packed >> (N*8));
}

template <int N>
int8_t unpack_s(uint32_t packed) {
    uint8_t r = unpack_u<N>(packed);
    return (r <= 127 ? r : r - 256); // thanks to caf
}

int main() {
    uint32_t x = pack(4,5,6,-7);
    std::cout << (int)unpack_u<0>(x) << "\n";
    std::cout << (int)unpack_s<1>(x) << "\n";
    std::cout << (int)unpack_u<3>(x) << "\n";
    std::cout << (int)unpack_s<3>(x) << "\n";
}

Output:

4
5
249
-7

This is as portable as the uint32_t, uint8_t and int8_t types. None of them is required in C99, and the header stdint.h isn't defined in C++ or C89. If the types exist and meet the C99 requirements, though, the code will work. Of course in C the unpack functions would need a function parameter instead of a template parameter. You might prefer that in C++ too if you want to write short loops for unpacking.

To address the fact that the types are optional, you could use uint_least32_t, which is required in C99. Similarly uint_least8_t and int_least8_t. You would have to change the code of pack_helper and unpack_u:

uint_least32_t mask(uint_least32_t x) { return x & 0xFF; }

uint_least32_t pack_helper(uint_least32_t c0, uint_least32_t c1, uint_least32_t c2, uint_least32_t c3) {
    return mask(c0) | (mask(c1) << 8) | (mask(c2) << 16) | (mask(c3) << 24);
}

template <int N>
uint_least8_t unpack_u(uint_least32_t packed) {
    // cast to avoid potential warnings for implicit narrowing conversion
    return static_cast<uint_least8_t>(mask(packed >> (N*8)));
}

To be honest this is unlikely to be worth it - chances are the rest of your application is written on the assumption that int8_t etc do exist. It's a rare implementation that doesn't have an 8 bit and a 32 bit 2's complement type.

Answer 5

You could also let the compiler do the work for you.

union packedchars {
  struct {
    char v1,v2,v3,v4;
  }
  int data;
};

packedchars value;
value.data = 0;
value.v1 = 'a';
value.v2 = 'b;

Etc.

Answer 6

I liked Joey Adam's answer except for the fact that it is written with macros (which cause a real pain in many situations) and the compiler will not give you a warning if 'char' isn't 1 byte wide. This is my solution (based off Joey's).

inline uint32_t PACK(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) {
    return (c0 << 24) | (c1 << 16) | (c2 << 8) | c3;
}

inline uint32_t PACK(sint8_t c0, sint8_t c1, sint8_t c2, sint8_t c3) {
    return PACK((uint8_t)c0, (uint8_t)c1, (uint8_t)c2, (uint8_t)c3);
}

I've omitted casting c0->c3 to a uint32_t as the compiler should handle this for you when shifting and I used c-style casts as they will work for either c or c++ (the OP tagged as both).