-3

I need to serialise a struct and I am trying to do this using memcpy. But it is not working. I can tell by looking at the byte stream - I see garbage characters. Why?

Also I get runtime error:

Run-Time Check Failure #2 - Stack around the variable 'addresses' was corrupted.

What is happening and how can I fix this?

I am using #pragma pack(push, 1) which I thought would mean there would be no padding of the structs.

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

#pragma pack(push, 1)  /* padding has to be disabled for casting to struct to work at other end */
typedef struct {
    uint8_t             start_char; 
    uint8_t             msg_type;      
    uint8_t             length;     
} MSG_HEADER;

typedef struct {
    uint8_t         denomination[6];   
    uint8_t         path;    
    uint8_t         min_level; 
    uint16_t        max_level;     
    uint16_t        weight;    
    uint8_t         address;  
} CONFIG_DATA;

typedef struct {
    MSG_HEADER            header;
    uint8_t               clear_type;  
    CONFIG_DATA           config_data[12]; 
    uint8_t               system_algorithm; 
    uint8_t               max_transaction;   
} MSG_CONFIGURATION;
#pragma pack(pop) /* only affect this file */

typedef struct {
    unsigned char data[256];
    size_t length;
    int msg_type;
} TCHU_MESSAGE;

enum DRM_MESSAGE_TYPE { 
    CONFIG, CLEAR_COUNT, DISPENSE, CANCEL_TRANSACTION };

void TestCopy()
{
    MSG_CONFIGURATION config;

    config.clear_type = 0;  
    config.system_algorithm = 0;
    config.max_transaction = 17;

    const int NumItems = 12;
    const uint16_t maxLevel = 300;

    static const char* denoms[] = { "GB005A","GB005B","GB010A","GB010B",
        "GB020A","GB050A","GB050B","GB100A",
        "GB100B","GB200A", "EU100A", "EU100B" };

    const uint8_t addresses[] =     { 0, 0, 5, 5, 0, 7, 7, 8, 8, 9, 0, 0 };
    const uint8_t sorting_paths[] = { 5, 5, 4, 4, 5, 2, 2, 1, 1, 3, 0, 0 };

    for(int i = 0; i < NumItems; ++i) {
        memcpy(config.config_data[i].denomination, denoms[i], 6);
        config.config_data[i].address = addresses[i];
        config.config_data[i].path = sorting_paths[i];
        config.config_data[i].min_level = 3;
        config.config_data[i].max_level = maxLevel;
        config.config_data[i].weight = 1000;
    }

    config.header.start_char = 1;
    config.header.msg_type = 2;
    config.header.length = sizeof(MSG_CONFIGURATION);

    TCHU_MESSAGE tchu_msg = {0};

    // why does the memcpy not work?  How can I get it to work?
    memcpy(tchu_msg.data, &config+sizeof(MSG_HEADER), sizeof(MSG_CONFIGURATION) - sizeof(MSG_HEADER));

    printf("sizeof(MSG_HEADER) = %u\n", sizeof(MSG_HEADER));
    printf("sizeof(MSG_CONFIGURATION) = %u\n", sizeof(MSG_CONFIGURATION));

    // get garbage in copyconfig
    MSG_CONFIGURATION copyconfig;
    memcpy(&copyconfig+sizeof(MSG_HEADER), tchu_msg.data, sizeof(MSG_CONFIGURATION) - sizeof(MSG_HEADER));

    if(copyconfig.header.start_char != config.header.start_char)
    {
        // we get to here
        printf("mismatch between original and copy\n");
    }
}

int main() {

    TestCopy();

    // I also get Run-Time Check Failure #2 - Stack around the variable 'addresses' was corrupted.
    // when program ends
}
Paul R
  • 208,748
  • 37
  • 389
  • 560
Angus Comber
  • 9,316
  • 14
  • 59
  • 107
  • 4
    Welcome to Stack Overflow! It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Paul R Apr 27 '16 at 11:38
  • 1
    Is it wise to offset into structs like that? Is the ordering of the members guaranteed by C? For simplicity sake, could you not remove MSG_HEADER from MSG_CONFIGURATION and send them separately, and avoid offsetting into structs like that. –  Apr 27 '16 at 11:43
  • 3
    Do you know what `&config+sizeof(MSG_HEADER)` does? It advances the address `&config` by `sizeof(MSG_HEADER)*sizeof(MSG_CONFIGURATION)` bytes. I don't think that's what you want. – user694733 Apr 27 '16 at 11:45
  • 1
    You don't serialise, but just reinterpret. Use correct serialisation with bitops&shifts. And the C standard does not define the behaviour of that pragma (or most others) – too honest for this site Apr 27 '16 at 11:47

1 Answers1

4

My compiler instantly told me what was wrong:

warning: '__builtin___memcpy_chk' will always overflow destination buffer [-Wbuiltin-memcpy-chk-size]
    memcpy(&copyconfig+sizeof(MSG_HEADER), tchu_msg.data, sizeof(MSG_CONFIGURATION) - sizeof(MSG_HEADER));

Why is that? Well, let's look at the destination:

&copyconfig + sizeof(MSG_HEADER)

That means "Take the address of copyconfig, treat it as an array, and take the Nth object where N is sizeof(MSG_HEADER). I think you thought it would add N bytes, but it actually adds N instances of MSG_CONFIGURATION. Instead, use this: 

&copyconfig.header + 1

That is, "Take the address of copyconfig.header and go to just beyond it."

You could equally do this:

(char*)&copyconfig + sizeof(MSG_HEADER)

Because the size of one char is one byte. Or, since your struct is packed:

&copyconfig.clear_type

Because that's the address of the first byte you actually want to copy into.

For more details, read: Pointer Arithmetic .

Community
  • 1
  • 1
John Zwinck
  • 239,568
  • 38
  • 324
  • 436