Writing to an allocated 2D array

Question

I am trying to create a struct with a variable length array of fixed size strings.

struct foo_query{
  char tag[10];
  int value_count;
  char * values[VAL_SIZE];
};

Now, I want to create an array of these structs, and allocate some memory for the values:

#define buffer(a) (char *) malloc(sizeof(char[a][VAL_SIZE]))
foo_query queries[total_queries] = {
    {"FOO", 25, buffer(25)},
    {"BAR", 21, buffer(21)}
};
#undef buffer

Finally, I want to actually write some data to the values.

query_index = 0;
for(int i = 0; i < queries.value_count; i++){
    strncpy(queries[query_index].values[i], "Hello", VAL_SIZE);
    Serial.outln("success");
}

But that last bit fails. success is printed once or twice, and then everything just stops.

As far as I can tell, the memory has been allocated and I'm not overflowing anything, so why is the code crashing?

I know similar questions have been asked, like [this one](https://stackoverflow.com/questions/12462615/how-do-i-correctly-set-up-access-and-free-a-multidimensional-array-in-c?noredirect=1&lq=1), but I have done what it says on that question, and my code still does not work. — Mathieu pasquet, Oct 31 '17 at 11:43
I'm not sure I see why that's relevant in this case. The problems are occurring on write, not read ( In fact I never read from the array ) so NULL termination shouldn't make a difference. — Mathieu pasquet, Oct 31 '17 at 11:53
you haven't said on which line the code crashes, so we're left guessing what the problem might be. — Chris Turner, Oct 31 '17 at 12:02
Sorry if it's ambiguous. I'm running on an embedded system, so I'm also guessing what the problem is. It looks like some kind of buffer overflow, because as I said in the question, success is printed once or twice and then everything stops. So I suspect that the buffer allocation that I'm doing is not actually working as I expect it to. — Mathieu pasquet, Oct 31 '17 at 12:06

score 0 · Answer 1 · answered Oct 31 '17 at 13:44

Basically your memory allocating is wrong.

foo_query queries[total_queries] = {
    {"FOO", 25, buffer(25)},
    {"BAR", 21, buffer(21)}
};

When you create queries above, you're only populating the first element of values because the way you've defined it, values is an array of char *.

When you come to put values into that array if i is more than 0, you're copying into an unassigned pointer.

strncpy(queries[query_index].values[i], "Hello", VAL_SIZE);

When i is more than 0, queries[query_index].values[i] could be NULL or it could be pointing to some part of memory. Either way, it'll lead to undefined behaviour and your code crashing randomly.

Given you're trying to copy VAL_SIZE characters into it though, it looks more likely that you've actually defined values incorrectly. Rather than an array of char *, it looks more likely you're looking for a pointer to a char array of VAL_SIZE? If that is the case you need to define it as:

char (* values)[VAL_SIZE];

It would also fit with the rather odd amount of memory you're allocating with malloc which would look more clear as malloc(sizeof(char[VAL_SIZE})*(a)). (Note: always wrap macro parameters in brackets to ensure you don't have crazy precedence issues ie buffer(10+10) turns into malloc(sizeof(char[VAL_SIZE})*10+10) without the brackets).

You should still add in

queries[query_index].values[i][VAL_SIZE-1]='\0';

after the strncpy just to make sure that if the string being copied is too large to fit in, that your strings are NUL terminated too.

Writing to an allocated 2D array

1 Answers1