c portable address encoding-CodePudding

I'm writing a software that, at some point must write internal addresses into a buffer. I wrote the following code which works. But produce warnings when cross-compiling to a target device with an address size smaller than 64 bits.

How can I make this portable without generating errors? I would have expected gcc to ignore the warning due to the condition-always-false around the problematic statement. I get the same behaviour when doing this with a template (where i feed sizeof(void*()) as address_size).

uint8_t decode_address_big_endian(uint8_t* buf, uintptr_t* addr)
    {
        constexpr unsigned int addr_size = sizeof(void*);
        static_assert(addr_size == 1 || addr_size == 2 || addr_size == 4 || addr_size == 8, "Unsupported address size");

        uintptr_t computed_addr = 0;
        unsigned int i = 0;

        if (addr_size >= 8)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 56));
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 48));
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 40));
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 32));
        }

        if (addr_size >= 4)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 24));
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 16));
        }

        if (addr_size >= 2)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 8));
        }

        if (addr_size >= 1)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 0));
        }
        
        *addr = computed_addr;

        return static_cast<uint8_t>(addr_size);
    }

The code works. MSVC is able to optimize that to a single x86_64 instruction. It also works on a AtMega328p, but avr-gcc do throw these warnings.

/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:32:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 56));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:33:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 48));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:34:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 40));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:35:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 32));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:40:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 24));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:41:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << 16));
                                                                        ^

In this case, I can make a static_assert on address_size to prove that it is indeed equal to 2. But still get the warnings.

CodePudding user response：

Try

if constexpr (addr_size >= 8)

If C 17 is not available to you, you can suppress the warning

if (addr_size >= 8) {
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << ((addr_size >= 8) * 56)));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << ((addr_size >= 8) * 48)));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << ((addr_size >= 8) * 40)));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << ((addr_size >= 8) * 32)));
}

If addr_size >= 8 is true, it evaluates to 1 in the multiplication arguments. Otherwise, it would evaluate to 0, but the branch is not executed.

Shorter with @RaymondChen's hint.

if (addr_size >= 8) {
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << (56 % (addr_size * 8));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << (48 % (addr_size * 8));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << (40 % (addr_size * 8));
   computed_addr |= ((static_cast<uintptr_t>(buf[i  ]) << (32 % (addr_size * 8));
}
// Will be shorter with constexpr unsigned int addr_size = sizeof(void*) * 8;

Alternative code

if (addr_size >= 8) {
   uint64_t tmp;
   tmp = static_cast<uintptr_t>(buf[i  ]) << 28;
   computed_addr |= tmp << 28;
   tmp = static_cast<uintptr_t>(buf[i  ]) << 24;
   computed_addr |= tmp << 24;
   tmp = static_cast<uintptr_t>(buf[i  ]) << 20;
   computed_addr |= tmp << 20;
   tmp = static_cast<uintptr_t>(buf[i  ]) << 16;
   computed_addr |= tmp << 16;
}

CodePudding user response：

I would forgo bitpacking and just do an in-place conversion of bytes on little-endian devices.

To deserialize an address from big-endian, you only have to do this:

size_t decode_address_big_endian(uint8_t* buf, uintptr_t* addr)
{
    const size_t address_size = sizeof(uintptr_t);
    uint32_t test = 0x01020304;
    bool isLittleEndian = (htonl(test) != test); // isLittleEndian = (1 != *(uint8_t*)&test));


    if (isLittleEndian)
    {
        uint8_t tmp[sizeof(uintptr_t)];
        memcpy(tmp, buf, address_size);
        for (size_t i = 0; i < address_size/2; i  )
        {
            std::swap(tmp[i], tmp[address_size-1-i]);
        }
        memcpy(addr, tmp, address_size);
    }
    else
    {
        memcpy(addr, buf, address_size);
    }
    return address_size;
}