I've been trying to write a function to decode CAN bytes in C, taking into account endianness of values coming in and endianness of system. Now, it works fine for unsigned values, but sucks for signed ones.
I have a feeling I have deeply misunderstood how signed representation works in C - I assumed the MSB was the sign flag for signed data (ie last byte of Little Endian, first byte of Big Endian). Could anybody take a look at my function below and let me know what i've done wrong?
/**
* @brief can_interact_decode - converts array of length x containing hex bytes into a uint64
* @param[in] const uint8_t* - const array of hex bytes
* @param[in] const size_t - length of hex bytes array
* @param[in] const enum can_interact_signedness - whether the bytes are storing a signed value or not. SIGNED_VAL indicates signed, UNSIGNED_VAL indicates unsigned
* @param[in] const enum can_interact_endianness - endianess. LITTLE_ENDIAN_VAL is little, BIG_ENDIAN_VAL is big
* @return[out] uint64_t - interpretted value as unsigned int from hex bytes, taking other params into account
*/
uint64_t can_interact_decode(const uint8_t *payload, const size_t data_len, const enum can_interact_signedness is_signed, const enum can_interact_endianness byte_order)
{
uint64_t result; /* [0,0,0,0,0,0,0,0] */
uint8_t* blocks; /* array of 8 */
result = 0;
blocks = (uint8_t*)(&result);
if(byte_order == LITTLE_ENDIAN_VAL) {
memcpy(blocks, payload, (is_signed ? data_len - 1 : data_len));
blocks[7] = is_signed ? payload[data_len - 1] : blocks[7];
result = le64toh(result); /* little endian->host byte order */
} else if(byte_order == BIG_ENDIAN_VAL) {
memcpy(blocks (8 - data_len) (is_signed ? 1 : 0), (is_signed ? payload 1 : payload), (is_signed ? data_len - 1 : data_len));
blocks[0] = is_signed ? payload[0] : blocks[0];
result = be64toh(result); /* big endian->host byte order */
}
return result;
}
CodePudding user response:
Problems:
Sign extending woes
OP appears to want to sign extend the sign bit into other bytes.
if(byte_order == LITTLE_ENDIAN_VAL) {
//memcpy(blocks, payload, (is_signed ? data_len - 1 : data_len));
//blocks[7] = is_signed ? payload[data_len - 1] : blocks[7];
memcpy(blocks, payload, data_len);
if (is_signed && ((const int8_t*)payload)[data_len - 1] < 0) {
memset(blocks data_len, 0xFF, 8 - data_len);
}
result = le64toh(result); /* little endian->host byte order */
I will leave similar changes in else if(byte_order == BIG_ENDIAN_VAL)
block for OP to do.