0

I need to write data into a structure where the length of the data depends on the command I want to send to a device. For that I have defined the following structure:

typedef struct {
    uint8 len;          // Command length (cmd ... crc)
    uint8 cmd;          // Command code
    uint8 data_length;  // Data length
    uint8 data[12];     // Data: max 12 Byte
    uint8 crc_h;        // CRC value MSB
    uint8 crc_l;        // CRC value LSB
}CMD_TYPE;

Note: the members cmd, *data_length* and crc that are always present, instead member data can be empty or contains up to 12 Bytes.

I have created a function that returns a initialized command according to the parameters passed to the function:

CMD_TYPE Device::get_cmd(uint8 cmd, uint8 data_len, uint8 *data)
{
    CMD_TYPE cmd;

    cmd.len = (4 + data_len) * sizeof(uint8);
    cmd.cmd = cmd;
    cmd.data_length = data_len;
    cmd.data = (uint8 *)realloc(cmd.data, data_len*sizeof(uint8));
    if(data_len > 0)    memcpy(cmd.data, data, data_len);

    add_crc16((uint8*)&cmd);

    return cmd;
}

The function get_cmd() is used like this:

uint8 cmd_code = 0x01;
uint8 data[2] = {0xAB, 0xCD};

CMD_TYPE cmd = local_device->get_cmd(cmd_code, 2, data);
retVal = local_device->send(cmd);

When I try to compile this code I get an error from the compiler for that line:

cmd.data = (uint8 *)realloc(cmd.data, data_len*sizeof(uint8));

and the compiler error is:

error: lvalue required as left operand of assignment

The aim of using realloc() is to re-size the array data or to remove it at all from my new command structure. What is wrong in my code? Is that the right way to initialize structures with dynamic memory allocation?

1
  • 2
    Arrays are not pointers - you can't reassign the memory position of an array. Commented Sep 9, 2011 at 14:43

3 Answers 3

2

What you want is the infamous struct hack:

typedef struct
{
    uint8   len;          // Command length (cmd ... crc)
    uint8   cmd;          // Command code
    uint8   data_length;  // Data length
    uint8   crc_h;        // CRC value MSB
    uint8   crc_l;        // CRC value LSB
    uint8   data[1];      // Data: max 12 Byte
} CMD_TYPE;

The trick is to allocate enough room for all of the members of the struct up to data[], then add enough bytes for the data[] member:

CMD_TYPE * allocCmd(int dataSize)
{
    int         len;
    CMD_TYPE *  p;

    len = sizeof(CMD_TYPE) + (dataSize-1)*sizeof(uint8);
    p = (CMD_TYPE *) malloc(len);
    memset(p, 0, len);
    p->data_length = dataSize;
    return p;
}

Here, len is calculated to be the size of the struct, minus the size of the empty data member, plus however many elements dataSize specifies for the data array.

The catch is that you have to be careful never to access any elements of p->data[] beyond what is actually allocated in it (inside the struct).

Sign up to request clarification or add additional context in comments.

4 Comments

Yes, a structure like this would be easier.. but the problem is that I have to send data exactly in the order that I have in my structure.
@imx51, you can only vary the data at the end of the structure. You cannot vary the size of the data inside the structure. You can make data a pointer to another structure/array of a varying length (as the pointer itself won't change size)
You could also use this struct hack and consider the CRC values to be the last two bytes of the data, instead of having separate fields for them.. since you have the length in the structure, it'd be easy to figure out the indices for them... if you're planning to just dump the memory occupied by the struct to send it. If you're not, then you don't need to send the fields in the same order they appear in the struct.
i have been using this for a while , didn't quite realize that its a formal notion , part of a standard now ! wow, genius me :D
1

Your CMD_TYPE.data is an array, not a pointer. Since you want it to track dynamically allocated memory, it has to be a pointer:

uint8_t * data;

Just don't forget to initialize it with malloc() (or by setting it to zero before realloc()) and to clean up after yourself

By the way, do not cast the result of malloc() and co.

7 Comments

Why shouldn't he cast the result of malloc()? It returns a void *, no?
Because in C you don't cast - you might be hiding errors if you do, and C explicitly allows you to use void pointers like that. Plus, doesn't it just look plain ugly?
Thanky you for your very fast reply! I have made the change and now when data_length is 0, everything seems to be correct. But when I want to write data into the field data, looks like that data are not copied properly. Here my changes: cmd.data = (uint8 *)malloc(data_len * sizeof(uint8)); Note: if I NOT cast the result, the compiler returns the error: error: invalid conversion from 'void*' to 'uint8*'
@imx: you'll need some additional code now to initialize the struct correctly and to clean it up. It's not just the one line of change.
Perhaps you were not being upfront to us about the language you use. C (I'm referring to "The ISO C Standard") expressly permits conversions from void pointer. If you're using some non-standard compiler or dialect (Borland Turbo C from 1985? MS Visual C++?), please tag the question appropriately. Note that C++ is not the same as C, and that Microsoft doesn't have a real C compiler.
|
0

array defined as a[..] are immutable, you can't assign anything to them. Instead you should use pointers.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.