Variable length arrays in struct

Question

I'm writing an application in C (as a beginner) and I'm struggling with getting corrupted data inside a struct that contains a variable length array. I found similar issues described in forum posts on cprogramming.com and also on cert.og/secure-coding. I thought I'd had found the right solution, but it seems not.

The struct looks like this;

typedef struct {
    int a;
    int b;
} pair;

typedef struct {
    CommandType name;
    pair class;
    pair instr;
    pair p1;
    pair p2;
    pair p3;
    CommandType expected_next;
    char* desc;
    int size;
    pair sw1;
    pair sw2;
    pair* data;
} command;

With the problematic one being "command". For any given instance (or whatever the correct phrase would be) of "command" different fields would be set, although in most cases the same fields are set albeit in different instances.

The problem I have is when trying to set the expected_next, name, sw1, sw2, size and data fields. And it's the data field that's getting corrupt. I'm allocating memory for the struct like this;

void *command_malloc(int desc_size,int data_size)
{
    return malloc(sizeof(command) +
                  desc_size*sizeof(char) +
                  data_size*sizeof(pair));
}

command *cmd;
cmd = command_malloc(0, file_size);

But when I (pretty) print the resulting cmd, the middle of the data field appears to be random garbage. I've stepped through with gdb and can see that the correct data is getting loaded into the the field. It appears that it's only when the command gets passed to a different function that it gets corrupted. This code is called inside a function such as;

command* parse(char *line, command *context)

And the pretty-print happens in another function;

void pretty_print(char* line, command* cmd)

I had thought I was doing things correctly, but apparently not. As far as I can tell, I construct other instances of the struct okay (and I duplicated those approaches for this one) but they don't contain any variable length array in them and their pretty-prints looks fine - which concerns me because they might also be broken, but the breakage is less obvious.

What I'm writing is actually a parser, so a command gets passed into the parse function (which describes the current state, giving hints to the parser what to expect next) and the next command (derived from the input "line") is returned. "context" is free-d at the end of the parse function, which the new command getting returned - which would then be passed back into "parse" with the next "line" of input.

Can anyone suggest anything as to why this might be happening?

Many thanks.

Solution

When you allocate memory to structure, only a pointer size gets allocated to *desc. You must allocate memory to the space (array contents) desc points to, as someone already pointed out. Purpose of my answer is to show slightly different way of doing that. Since having a pointer *desc increases structure size by a word (sizeof pointer), you can safely have a variable length array hack in you structure to reduce structure size.

Here's how your structure should look like, notice that desc[] has been pulled down to the end of structure :

typedef struct {
    CommandType name;
    pair class;
    pair instr;
    pair p1;
    pair p2;
    pair p3;
    CommandType expected_next;
    int size;
    pair sw1;
    pair sw2;
    pair* data;
    char desc[];
} command;

Now, 1. Allocate memory for command which includes array size also :

 command *cmd = malloc(sizeof(command) + desc_length);

1. Use desc :

   cmd->desc[desc_length -1] = '\0';

This hack works only if member is at the end of structure, saves structure size, saves pointer indirection, can be used if array length is structure instance specific.