CODEWITHSUNDEEP
cstructsegmentation-faultmallocdynamic-arrays
joshuatvernon
joshuatvernon

Reputation: 1621

How to correctly allocate memory to a dynamic array of integers stored in a struct?

I have a function that sets values to a struct:

My struct:

struct entry {
    char key[MAX_KEY];
    int* values;
    size_t length;
    entry* next;
    entry* prev;
};

My function:

// Sets entry values
void command_set(char **commands, int arg_num) {
    struct entry e;
    e.length++;
    strcpy(e.key, commands[1]);
    for (int i = 2; i < arg_num; i++) {
        e.values[i - 2] = atoi(commands[i]);
    }
}

where:

I run the code and I get a segmentation fault 11. I have narrowed it down to the line:

e.values[i -2] = atoi(commands[i]);

I assume that I have to use malloc to allocate memory as I don't appear to have gone out of bounds with my loop. I have tried to understand the correct way to allocate memory however I can't seem to get the syntax correct for allocating sizeof(int) to a dynamic array of integers.

I have tried:

e.values[i - 2] = malloc(sizeof(int));

and

e.values[i - 2] = (int) malloc(sizeof(int));

and

e.values[i - 2] = malloc(sizeof(int *));

However I get the error:

incompatible pointer to integer conversion assigning
  to 'int' from 'void *' [-Werror,-Wint-conversion]

Upvotes: 0

Views: 188

Answers (3)

Uysses
Uysses

Reputation: 31

as your struct is as follows 

struct entry {
    char key[MAX_KEY];
    int* values;
    size_t length;
    entry* next;
    entry* prev;
};

then you should allocate memory to it as 

e.values =(int *)malloc(arg_num*sizeof(int));

like if you have 10 values then you are allocating 10*4 values to it. and invoke free on it 

free(e.values)

when the e or e.values is no more useful. for more information you can see here

Upvotes: 2

shafeeq
shafeeq

Reputation: 1599

Modify the function as below.

void command_set(char **commands, int arg_num) {
    struct entry e;
    e.length++;
    strcpy(e.key, commands[1]);
    //here is the memory allocation
    e.values = malloc(arg_num-1 * sizeof(int));
    for (int i = 0; i < arg_num-1; i++) {
        e.values[i] = atoi(commands[i+1]);
    }
}

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409482

You must allocate the whole array:

e.values = malloc(sizeof(int) * (arg_num - 2))

Important: Remember to call free when you're done with the memory or you will have a memory leak.

You have another problem though, unrelated to the one you're asking about.

You do

struct entry e;
e.length++;

When the structure object e is defined, it is uninitialized, all its members will have an indeterminate value. Using such uninitialized data in any way except to initialize it will lead to undefined behavior. And you do use such uninitialized values when you do e.length++.

That increase simply doesn't make any sense in the code as you show it. On the other hand, that function doesn't make a lot of sense anyway since the variable e and all its data will simply "disappear" when the function returns. So I can only assume that it's not the complete function you show us.

To initialize the structure to all zeroes, simply do

struct entry e = { 0 };

Upvotes: 4

Related Questions