Do not access a variable through a pointer of an incompatible type?

Question

I write some code like this:

typedef struct {
    uint8_t ms;
    uint8_t op;
    uint16_t len;
} DATA_HEAD;

static int data_rcv(uint8_t *rdt)
{
uint16_t op;
int op_king;
DATA_HEAD *pH;

pH = (DATA_HEAD *)rdt;
op_king= data_check(pH->op);
}

static int data_check(uint8_t op) {
//some code
}

I build and get this warning:

Accessing an object through a pointer pH whose type DATA_HEAD* is incompatible with the type of the object. Do not access a variable through a pointer of an incompatible type.

How to fix this warning? Thanks very much!

Answer 1

What you try to archive is called "pointer punning". It generally should be avoided at any cost.

The safest method of punning types is use of the memcpy function.

typedef struct {
    uint8_t ms;
    uint8_t op;
    uint16_t len;
} DATA_HEAD;

static int data_rcv(void *rdt)
{
    int op_king;
    DATA_HEAD pH;

    memcpy(&pH, rdt, sizeof(pH));
    op_king= data_check(pH.len);
    return op_king;
}

memcpy function is well known to most modern optimizing compiler and in most circumstances, it will not be called.

In the example above the compiler will generate a simple load from the memory:

data_rcv:
        movzx   edi, WORD PTR [rdi+2]
        xor     eax, eax
        jmp     data_check

But if the architecture requires for example aligned access to some datatypes (like Cortex-M0)

data_rcv:
        push    {r4, lr}
        sub     sp, sp, #8
        add     r4, sp, #4
        movs    r1, r0
        movs    r2, #4
        movs    r0, r4
        bl      memcpy
        ldrh    r0, [r4, #2]
        bl      data_check
        add     sp, sp, #8
        pop     {r4, pc}

Protecting you from HardFaults if accesses are not properly aligned.

Answer 2

How to fix this warning?

Supposing based on the function name and parameter type that all you have to start with is a byte array, and that you are supposing that the first four bytes map to the members of a DATA_HEAD (including correct byte order for op), the simplest thing to do for your particular example is to pick out the wanted bytes directly:

static int data_rcv(uint8_t *rdt) {
    int op_king = data_check(rdt[1]);
}

The answer is similar if in reality you actually need to construct a complete DATA_HEAD for some other use:

static int data_rcv(uint8_t *rdt) {
    DATA_HEAD dh = { .ms = rdt[0], .op = rdt[1] };
    memcpy(&dh.len, rdt + 2, sizeof(dh.len));

    int op_king = data_check(dh.op);

    // ...
}

Or if you need a DATA_HEAD whose lifetime extends beyond return from data_rcv(), then you can dynamically allocate it and set the members in similar manner.

What you cannot do (without eliciting the warning you present or a similar one), is reinterpret part of an array of bytes in-place as an object that is neither of character type nor of array-of-character type.

Answer 3

I'm not quite sure what you're trying to do here, but I'm guessing the argument to the data_rcv function should take in a DATA_HEAD object or pointer, not a uint8_t. You should almost never be converting between pointers to completely different types like this. Also, there's no need to declare your variables before you use them; you can just do int op_king = data_check(...).

I see you've tagged your question with C++, in which a more idiomatic way for this kind of code is using references. Anyway, hope this helps.

Answer 4

Generally there is a rule in C which allows us to inspect any type by using a character pointer (uint8_t* can normally be assumed to be one) and then access it byte by byte, to get the raw binary representation.

This cannot be done the other way around - you can't take something like an array of uint8_t and convert it to a struct. An uint8_t* and DATA_HEAD* are not compatible pointer types - though when you add an explicit cast as in your code, C allows the pointer conversion itself. The C language actually allows us to convert between all manner of wildly incompatible pointer types. Though in some hardware, doing so could cause instruction traps - it is implementation-defined behavior what will happen.

By adding the cast you tell the compiler "shut up, I (don't) know what I'm doing!". Which is why the message like the one you quoted can only come from a static analyser, which does more in-depth checks for bug sources.

Now the real problems appear when you de-reference the data:

• Depending on where the uint8_t* points at, accessing it by an incompatible larger type could be a misaligned access. Some systems can handle such, some can't. Some systems can handle it but produce slower code.
• Even if the uint8_t* pointer happens to point at an aligned address, a struct like the one in your example is very likely to contain padding bytes, to ensure that the struct itself is always aligned. If you copy a raw character array of 100% data into such a struct, you will be writing data into padding bytes and lots of data will end up in the wrong place.
• The C type system does not allow you to re-interpret what uint8_t* points at as a DATA_HEAD type unless the pointer actually happens to be pointing at such a struct allocated elsewhere. If it doesn't, this is a so-called "strict aliasing violation" What is the strict aliasing rule? - an undefined behavior bug where you de-reference pointers in a way which isn't supported by the underlying type system.

For these two reasons, the tool is correct to call out this fishy pointer conversion.

Rule of thumb: never covert between different pointer types by means of a cast unless you actually have in-depth knowledge of C. Otherwise you will likely be creating a lot of subtle but severe bugs.

Possible solutions:

• memcpy the data into a struct object, though in that case you have to be aware of potential struct padding.
• Assign the data to a struct object member by member.
• Change the function so that it takes a DATA_HEAD* then change the original uint8_t buffer into a union, along the lines of: typedef union { DATA_HEAD dh; uint8_t buf [n]; } foo;