Seg fault - Incorrect mallocing?

Question

When I try to use these malloc statements using these structs (these statements aren't all in a row in my actual code but built in to a function that mallocs/reallocs as needed) I believe my problem lies within these statements so I only included those, as I believe I am currently not mallocing correctly in order to get memory to store things in a struct of word_data_t inside an array data in a struct of type data_t inside an array in a struct of type index_data_t:

EDIT: Added compilable code.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

typedef struct word_data word_data_t;
typedef struct data data_t;
typedef struct index_data index_data_t;
#define INITIAL_ALLOCATION 2

void test();

struct word_data {

  int docunumber;
  int freq;
};

struct data {
  char *word;
  int total_docs;
word_data_t *data;
};

struct index_data {
  data_t *index_data_array;
};



int main(int argc, char const *argv[]) {
  test();
  return 0;
}

void test() {
  /* Inside a function called from main */
  char entered_word[4] = "wow";
  int docunum = 4, index=0, freq=6, current_size_outer_array=0, current_size_inner_array=0;
  int total_docs_in=1, doc_freq_pairs=1;
  index_data_t *index_data=NULL;
  for (index=0; index<4; index++) {
    index_data = (index_data_t*)malloc(sizeof(*index_data)*INITIAL_ALLOCATION);
    index_data->index_data_array = (data_t*)malloc(sizeof(*index_data->index_data_array)*INITIAL_ALLOCATION);
    current_size_outer_array = INITIAL_ALLOCATION;
    if (index == 2) {
      index_data->index_data_array = realloc(index_data->index_data_array, current_size_outer_array*sizeof(*(index_data->index_data_array)));
    }
    index_data->index_data_array[index].word=malloc(strlen(entered_word)+1);
    index_data->index_data_array[index].word=entered_word;
    index_data->index_data_array[index].data = (word_data_t *)malloc(sizeof(word_data_t)*INITIAL_ALLOCATION);
    current_size_inner_array = INITIAL_ALLOCATION;
    index_data->index_data_array[index].total_docs=total_docs_in;
    if (/* Need more data points */ doc_freq_pairs<2) {
      index_data->index_data_array[index].data = realloc(index_data->index_data_array[index].data, current_size_inner_array*(sizeof(*(index_data->index_data_array[index].data))));
    }
    index_data->index_data_array[index].data->docunumber = docunum;
    index_data->index_data_array[index].data->freq = freq;
  }
  printf("%d\n", index_data->index_data_array[0].total_docs);
  printf("%s\n", index_data->index_data_array[1].word);
  printf("%d\n", index_data->index_data_array[1].data->freq);
}

I get a seg fault, I cant seem to figure out why, what I expect to happen is the 2nd malloc call creates space for index_data->index_data_array[0] and [1], but maybe I need to set aside memory for them another way? this malloc stuff does my head in.

Thanks!

Answer 1

It was mentioned in a comment already, just stating it here for completeness: You should not cast return values from *alloc! Additionally: You SHOULD check all your retvals from *alloc for being NULL. Especially in the case of realloc, that means: void * tmp = realloc(old_ptr, new_size); if (!tmp) { error handling } else { old_ptr = tmp; }

So, now, let's go over a few problems:

==14011==    definitely lost: 96 bytes in 9 blocks
==14011==    indirectly lost: 176 bytes in 5 blocks

a. you enter the for loop and then inside it you initialize index_data IN EVERY ITERATION!. Probably, that first malloc should go outside of the for loop (this get's rid of the first 48 bytes of memory leak).

==14127==    definitely lost: 192 bytes in 9 blocks
==14127==    indirectly lost: 32 bytes in 2 blocks

Additionally, the first initialization of index_data->index_data_array should be done prior to the for loop too. Another 80 bytes of memory leak gone.

==14163==    definitely lost: 64 bytes in 7 blocks
==14163==    indirectly lost: 80 bytes in 3 blocks

b. Why?:

if (index == 2) {

You're counting the number of elements in your array index_data_array using current_size_outer_array. So use that one to check if there is still enough room.

if (index == current_size_outer_array) {
}

Also, then don't just realloc using that value again, but increase it before.

if (index == current_size_outer_array) {
    current_size_outer_array *= 2;
}

and use the right sizeof (which is the same, as in the initial malloc call above)

if (index == current_size_outer_array) {
    current_size_outer_array *= 2;
  void * tmp = realloc(index_data->index_data_array, sizeof(*index_data->index_data_array)*current_size_outer_array);
  if (!tmp) exit(2);
  index_data->index_data_array=tmp;
}

And ...

1
wow
6

viola

So, now this code is still leaking memory. In order for that to be fixed, you will have to make some free() calls.

Oh, and I you wonder, how I figured out the memory leaks: valgrind is your friend.

Here is the changed code, only the function test, rest remained unchanged:

void test() {
  /* Inside a function called from main */
  char entered_word[4] = "wow";
  int docunum = 4, index=0, freq=6, current_size_outer_array=0, current_size_inner_array=0;
  int total_docs_in=1, doc_freq_pairs=1;
  index_data_t *index_data=NULL;
  index_data = malloc(sizeof(*index_data)*INITIAL_ALLOCATION);
  index_data->index_data_array = malloc(sizeof(*index_data->index_data_array)*INITIAL_ALLOCATION);
  current_size_outer_array = INITIAL_ALLOCATION;
  for (index=0; index<4; index++) {
    if (index == current_size_outer_array) {
      current_size_outer_array *= 2;
      void * tmp = realloc(index_data->index_data_array, sizeof(*index_data->index_data_array)*current_size_outer_array);
      if (!tmp) exit(2);
      index_data->index_data_array=tmp;
    }
    index_data->index_data_array[index].word=malloc(strlen(entered_word)+1);
    index_data->index_data_array[index].word=entered_word;
    index_data->index_data_array[index].data = (word_data_t *)malloc(sizeof(word_data_t)*INITIAL_ALLOCATION);
    current_size_inner_array = INITIAL_ALLOCATION;
    index_data->index_data_array[index].total_docs=total_docs_in;
    if (/* Need more data points */ doc_freq_pairs<2) {
      index_data->index_data_array[index].data = realloc(index_data->index_data_array[index].data, current_size_inner_array*(sizeof(*(index_data->index_data_array[index].data))));
    }
    index_data->index_data_array[index].data->docunumber = docunum;
    index_data->index_data_array[index].data->freq = freq;
  }
  printf("%d\n", index_data->index_data_array[0].total_docs);
  printf("%s\n", index_data->index_data_array[1].word);
  printf("%d\n", index_data->index_data_array[1].data->freq);
}

Answer 2

Realloccing something you just mallocced (which you do twice) does not make sense to me, or anyone else, most likely.

Also, you should also strcpy a string to a buffer you malloced, not overwrite the buffer pointer with the one of the temporary const char * object.

Answer 3

First of all, like Michael Walz said: Please always include one SSCCE (http://sscce.org/), i.e. something, that compiles and demonstrates your problem. So putting everything after your comment "Inside a function called from main" in a newly created main function, adding the printf statement and compiling, I get 15 errors.

example.c:28:57: error: 'INITIAL_ALLOCATION' undeclared (first use in this function)
example.c:29:69: error: expected expression before '>' token
example.c:30:71: error: 'current_size_outer_array' undeclared (first use in this function)
example.c:31:2: error: expected ';' before 'index_data'
example.c:32:30: error: array subscript is not an integer
example.c:32:43: error: 'entered_word' undeclared (first use in this function)
example.c:33:30: error: array subscript is not an integer
example.c:34:30: error: array subscript is not an integer
example.c:34:54: error: 'word' undeclared (first use in this function)
example.c:35:30: error: array subscript is not an integer
example.c:35:66: error: expected expression before '>' token
example.c:35:45: error: too few arguments to function 'realloc'
example.c:36:2: error: expected ';' before 'index_data'
example.c:37:30: error: array subscript is not an integer
example.c:37:51: error: 'freq' undeclared (first use in this function)

There are additional warnings, but I don't care for them as long as the code doesn't even compile.

So, main question: What is INITIAL_ALLOCATION? 0? 1? 42?

Secondly: why malloc and then realloc right away? (I'm too slow, Julius already mentioned that too)

index_data->index_data_array = (data_t*)malloc(sizeof(*index_data- >index_data_array)*INITIAL_ALLOCATION);
index_data->index_data_array = realloc(index_data->index_data_array, current_size_outer_array*sizeof(*(index_data->index_data_array)))

What is current_size_outer_array?