1

How can I have a variable point to a member of a different struct? This is what I'm trying to do, but the third line fails.

volatile uint8_t tx_message_buffer[sizeof(MESSAGE)];
struct MESSAGE *tx_message = (MESSAGE *)tx_message_buffer;
struct PAYLOAD *tx_payload = (PAYLOAD *)tx_message->payload;

Here are the struct definitions.

#define MSG_MAX_PAYLOAD_LENGTH  64

typedef struct PAYLOAD {
    uint8_t descriptor;
    uint8_t parameters[MSG_MAX_PAYLOAD_LENGTH-1];
};

typedef struct MESSAGE {
    uint8_t address;
    uint8_t length;
    PAYLOAD payload;
    uint8_t checksum;
};
Oystein
  • 1,232
  • 11
  • 26
  • Side note: This is not good way do serialization. You can easily run in to problems with alignment and strict aliasing violation. It won't happen with this code as all types are `uint8_t`, but use any other types and you will have problems. (And I haven't mentioned padding and endianness issues yet). – user694733 Oct 31 '17 at 12:45
  • @user694733 Can you elaborate? I know I may have problems if e.g. using arrays, but what other types will cause problems? Do you have a better idea on how to gather all message elements in one type? – Oystein Oct 31 '17 at 12:47
  • Basically you cannot use casted pointer to one type to access data that is really of other type in C. There are exceptions to these rules for some cases but it really is a minefield. You can (and will) be bitten by compiler making assumptions that won't hold, or accidentally generating code that won't work reliably on your hardware. Correct way (most portable andreliable) is to convert each structure member manually to bytes by using bit shifting and masking and assigning results to byte array. – user694733 Oct 31 '17 at 12:54
  • @user694733 Can I trouble you to write a simple example of this? Not sure of the syntax of what you mean. – Oystein Oct 31 '17 at 12:58
  • 1
    Although not perfect, [this answer](https://stackoverflow.com/a/6002598/694733) (and other answers too on that question) has an short overview of the idea. – user694733 Oct 31 '17 at 13:02

3 Answers3

3

This code has many problems.

  • As pointed out in other answers, you cannot set a pointer to point at a PAYLOAD payload; member, you need to point at its address, &tx_message->payload.

  • typedef struct PAYLOAD {} should be typedef struct {} PAYLOAD.

  • (MESSAGE *)tx_message_buffer is a completely wild cast, which invokes several cases of poorly defined behavior. First of all, you should never cast away volatile qualifiers. But also, as soon as you de-reference this struct you will violate strict aliasing and invoke undefined behavior. Anything can happen.

    To solve these pointer bugs, you can do something similar to this:

    typedef struct {
        uint8_t address;
        uint8_t length;
        PAYLOAD payload;
        uint8_t checksum;
    } MESSAGE;
    
    typedef union {
      MESSAGE message;
      uint8_t tx_message_buffer[sizeof(MESSAGE)];
    } message_something;
    

    This code is valid and well-defined.

  • Using a struct to represent a data protocol is bad practice, as you must ensure that the struct contains no padding at all. The memory layout in your MESSAGE struct is by no means guaranteed to correspond to the memory layout of the data protocol. The struct may have padding bytes to suit the alignment requirements of the specific CPU.

    Disabling padding with non-standard C such as #pragma pack(1) may or may not be sufficient, depending on your portability requirements. To achieve full portability, you may have to write serialization/deserialization routines.

Lundin
  • 195,001
  • 40
  • 254
  • 396
2

You have a bigger issue in your code: the cast on the second line is not valid, because the storage for struct MESSAGE may generally have different alignment requirements than char[] array. For example, changing the type of descriptor to uint32_t could force an even-address location for the entire struct on some platforms.

Doing it the other way around would be valid, through, because you are allowed to convert any object pointer to char *:

volatile struct MESSAGE tx_message;
volatile uint8_t *tx_message_buffer = (char*)tx_message;

The third line fails because you did not take a pointer of PAYLOAD struct:

struct PAYLOAD tx_payload = &tx_message.payload;

There is no need to cast the result, because tx_message.payload is already of the correct type.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Why the parentheses? – Elazar Oct 31 '17 at 12:45
  • @Elazar I find address-of expressions slightly more readable with parentheses around non-trivial arguments. Obviously, `->` has higher precedence, so parentheses could be removed. – Sergey Kalinichenko Oct 31 '17 at 12:47
  • Getting error message: "Dereferencing pointer to incomplete type." – Oystein Oct 31 '17 at 12:50
  • @Oystein That's odd. Are you sure that the header for the two structs is included? – Sergey Kalinichenko Oct 31 '17 at 12:51
  • Yes, the include statement is right above the variable declarations. But when moving the MESSAGE and PAYLOAD to right after "typedef struct" (see OP), I'm still getting error message: "Initializer element is not constant". – Oystein Oct 31 '17 at 12:55
  • @Oystein Your sample must be missing some parts, because the modified code compiles and runs fine on ideone ([demo](https://ideone.com/plWTpb)). – Sergey Kalinichenko Oct 31 '17 at 13:00
  • 1
    @Oystein But then the declaration becomes invalid, because the address-of expression, indeed, is not constant, hence it is not suitable for initializers in static context. – Sergey Kalinichenko Oct 31 '17 at 13:14
  • @Oystein In a global context you could use `#define tx_payload (&tx_message.payload)` definition. It wouldn't be a pointer that you could re-point to some other payload, but it would behave like a pointer for reading and writing the content of the payload of `tx_message`. – Sergey Kalinichenko Oct 31 '17 at 13:16
  • Thank you. Could an apporoach be to declare the variables in a static context, but then define them by calling an init function that's using either the method in the original question or using your definition example? – Oystein Oct 31 '17 at 13:20
  • 1
    @Oystein Absolutely, if you have an init method for your program, you could definitely use it to set `tx_payload` pointer. – Sergey Kalinichenko Oct 31 '17 at 13:23
0

By using,

PAYLOAD payload;

You are getting a variable not a pointer. Meaning that

message->payload;

Is not a pointer.

You need to use a pointer.

PAYLOAD * payload;

Or get the address of the struct

&message->payload;
Rob
  • 1
  • 1