Casting void* to uint8_t* / uint16_t* properly in C

Viewed 1694

I created a function which prints the elements of a table. The table has always 256 elements and the type of element might be: uint8_t / uint16_t or uint32_t. I do an experiment with void argument in function.

void printf_crc_table(void *pt, uint8_t crc_length) {
    for (uint8_t i = 0; i < 16; i++) {
        for (uint8_t k = 0; k < 16; k++) {
            switch (crc_length) {
              case 16:
                printf("0x%04X, ", *(uint16_t*)pt);
                break;
              case 32:
                printf("0x%08X, ", *(uint32_t*)pt);
                break;
              default:
                printf("0x%02X, ", *(uint8_t*)pt);
                break;
            }
            pt++; //works only for uint8_t. For uint16_t it has to be pt+=2;
        }
        printf("\n");
    }
}

The problem I have is that increasing pointer address+1 works correctly only for uint8_t1 table.

If I call the function with uint16t_t element type, every second value is incorrect. This can be repaired by increasing the pointer's address by 2. But why? And how should I call/re-write the function that it always print correct values?

In main I call the function in a that way:

    printf_crc_table(crc8_0x07_t, 8);     // works fine
    printf_crc_table(crc16_0x1021_t, 16); // every second value is printed incorrectly.

I could use always uint32_t and cast simple uint8_t to uint32_t but I would like to use minimal resources of the micro-controller. Would it be better to use uint32_t instead of void?

3 Answers

Here you have a portable version. Do not worry about memcpy as it will be optimized out by compilers if platform hardware allows non aligned access.

#include <inttypes.h>

void printf_crc_table(const void *pt, const uint8_t crc_length)
{
    union
    {
        uint8_t  data8;
        uint16_t data16;
        uint32_t data32;
    }u32;
    const unsigned char *u8ptr = pt;

    for(uint8_t i=0;i<16;i++){
        for(uint8_t k=0;k<16;k++){
            switch (crc_length) {
                case 16:
                    memcpy(&u32.data16, u8ptr, sizeof(u32.data16));
                    printf("0x%04"PRIx16", ", u32.data16);
                    u8ptr += sizeof(u32.data16);
                    break;
                case 32:
                    memcpy(&u32.data32, u8ptr, sizeof(u32.data32));
                    printf("0x%08"PRIx32", ", u32.data32);
                    u8ptr += sizeof(u32.data32);
                    break;
                default:
                    
                    printf("0x%02"PRIx8", ", *u8ptr);
                    u8ptr++;
                    break;
            }
        }
        printf("\n");
    }
}

Resulting code: Two targets: x86 allows unaligned access and memcpy was removed by the compiler, Cortex M0 does not allow unaligned access and memcpy has not been optimized out. It is a safe and portable version.

https://godbolt.org/z/MzEWdE

If you do not want to use memcpy you can do it another way:

void printf_crc_table(void *pt,uint8_t crc_length)
{
    union
    {
        uint8_t  data8[sizeof(uint32_t)];
        uint16_t data16;
        uint32_t data32;
    }u32;
    unsigned char *u8ptr = pt;

    for(uint8_t i=0;i<16;i++){
        for(uint8_t k=0;k<16;k++){
            switch (crc_length) {
                case 16:
                    u32.data8[0] = *u8ptr++;
                    u32.data8[1] = *u8ptr++;
                    printf("0x%04"PRIx16", ", u32.data16);
                    break;
                case 32:
                    u32.data8[0] = *u8ptr++;
                    u32.data8[1] = *u8ptr++;
                    u32.data8[2] = *u8ptr++;
                    u32.data8[3] = *u8ptr++;
                    printf("0x%08"PRIx32", ", u32.data32);
                    break;
                default:
                    printf("0x%02"PRIx8", ", *u8ptr++);
                    break;
            }
        }
        printf("\n");
    }
}

Byte accesses are always aligned by definition. Modern compilers will replace them with the single access instruction if hardware supports unaligned access.

https://godbolt.org/z/eqnovf

Declaring the function with a void * argument is fine, but since it does not modify the data pointed to by this pointer, it should actually be declared as const void *. Casting the pointer on the fly as you is not very elegant and incrementing the pointer would be even more cumbersome. You could instead initialize pointers of the proper types and use these for both reading the value and incrementing.

This code does assume that the function is called with a properly aligned pointer pt if crc_length is 16 or 32.

Since you are concerned with code generation efficiency, you might want to check if defining loop index variables as int8_t generates smaller code than defining them as int.

As commented by chux, merely assigning the void * pointer to a pointer whose type does not match the effective type of the object has undefined behavior (on extremely rare architectures) so here is a modified version where the assignment is only performed for the specified crc_length:

#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>

void printf_crc_table(const void *pt, uint8_t crc_length) {
    for (uint8_t i = 0; i < 16; i++) {
        for (uint8_t k = 0; k < 16; k++) {
            switch (crc_length) {
              case 16: {
                  // assuming pt is properly aligned and points to an actual array of uint16_t
                  const uint16_t *p16 = pt;
                  printf("0x%04"PRIx16", ", *p16++);
                  pt = p16;
                  break;
                }
              case 32: {
                  // assuming pt is properly aligned and points to an actual array of uint32_t
                  const uint32_t *p32 = pt;
                  printf("0x%08"PRIx32", ", *p32++);
                  pt = p32;
                  break;
                }
              default: {
                  const uint8_t *p8 = pt;
                  printf("0x%02"PRIx8", ", *p8++);
                  pt = p8;
                  break;
                }
            }
        }
        printf("\n");
    }
}

The problem I have is that increasing pointer address+1 works correctly only for uint8_t1 table.

Well yes, if we accept that pt++ has the effect of increasing the address by 1 then it is to be expected that that will yield the correct result only if the data are 1 unit in size (i.e. the size of a char). For data whose elements are other sizes, increasing the address by 1 does not result in a pointer to the next element.

And perhaps this is a key point you are missing. void is not an automagical "the compiler will figure out the real type" placeholder. It is a specific type, albeit an incomplete one. And pointer arithmetic is defined in terms of the size of the pointed-to type.

That's a problem for your code: because pt is a pointer to void, an incomplete type, pointer arithmetic involving pt is not defined. Your claim in comments that your compiler does not even warn about that is surprising, and if it is true, then you would be well advised to turn up the warning level you request from it, or else choose a better compiler.

It would not be nearly as surprising for the compiler to accept the code with a warning, however, for some popular compilers implement an extension in which they treat arithmetic with void * as a special case, equivalent to arithmetic with char *.

I would recommend the approach in @chqrlie's answer, supposing the alignment issue to be unlikely to be of consequence in practice. If there is a risk that the data may not be correctly aligned for the element type then that would be something to reconsider.

For the record, however, here is a different variation on your original code that gets the pointer arithmetic right while cutting a little closer to your original:

void printf_crc_table(void *pt, uint8_t crc_length) {
    for (uint8_t i = 0; i < 16; i++) {
        for (uint8_t k = 0; k < 16; k++) {
            switch (crc_length) {
              case 16:
                uint16_t *p16 = pt;
                printf("0x%04X, ", *p16);
                pt = p16 + 1;
                break;
              case 32:
                uint32_t *p32 = pt;
                printf("0x%08X, ", *p32);
                pt = p32 + 1;
                break;
              default:
                uint8_t *p8 = pt
                printf("0x%02X, ", *p8);
                pt = p8 + 1
                break;
            }
        }
        printf("\n");
    }
}

To the extent that chqrlie's version has alignment issues, this has the same issues. The point being illustrated is that you can, with a little effort, maintain the data pointer as void * while still performing accesses and updates that are correct for the underlying element type.

Related