2

I wrote a piece of code to handle dynamic arrays. Idea was to use array of struct pointers, where the last member of array is NULL. Slight variation of code I wrote is below (using integers and not structures).

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>

void list_add(int **list, int* value) {
   for(int i = 0; true; i++) {
      if(list[i] == NULL) {
         list = realloc(list, (i+2) * sizeof(int*));
         list[i] = value;
         list[i+1] = NULL;
         break;
      }
   }
}

void list_init(int **list) {
   int* x;
   for(int i = 0; i < 100; i++) {
      x = malloc(sizeof(int));
      *x = i;
      list_add(list, x);
   }
}

int main() {
   int** l = malloc(sizeof(int*));
   l[0] = NULL;
   list_init(l);
}

While debugging, I discovered that only first 3 integers are added to the list. I can't seem to figure out why is this happening. Any ideas?

4
  • are you trying to allocate multi dimensional array ? (2 in this case?) Commented Aug 19, 2011 at 10:20
  • Is there some reason — other than wanting to use the NULL-terminator — to employ a storage structure that requires one malloc per int? This is highly inefficient and cumbersome. Commented Aug 19, 2011 at 10:24
  • Single letter variable names are bad, 'l', particularly so (looks like a one). Commented Aug 19, 2011 at 10:26
  • why (i+2) when reallocing ? Commented Aug 19, 2011 at 10:45

2 Answers 2

4

The problem is that the call to realloc() in list_add() potentially frees the memory block *list and allocates another. list_add updates its list pointer, but it does not return the updated pointer to the caller, list_init(); list_init()'s list pointer is potentially a pointer to the recently-freed memory block.

To fix this code, list_add() and list_init() need to be able to "return" the updated list pointer:

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>

void list_add(int ***p_list, int *value) {
   int **list = *p_list;
   int i;
   for(i = 0; true; i++) {
      if(list[i] == NULL) {
         list = realloc(list, (i+2) * sizeof(int*));
         list[i] = value;
         list[i+1] = NULL;
         break;
      }
   }
   *p_list = list;
}

void list_init(int ***p_list) {
   int **list = *p_list;
   int *x;
   int i;
   for(i = 0; i < 100; i++) {
      x = malloc(sizeof(int));
      *x = i;
      list_add(&list, x);
   }
   *p_list = list;
}

int main() {
   int **list = malloc(sizeof(int*));
   list[0] = NULL;
   list_init(&list);

   int **l = list;
   for (; *l != NULL; ++l) {
      printf("%d\n", **l);
   }
}

http://codepad.org/iGcSaJOR

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

Comments

1

EDIT

In this case of dynamic arrays the way you have told will not make anything better, the code will complicate only. For each addition of integer you have used realloc trying aggressively to save memory, but this will take more time while execution. Why not allocate a block of memory reserved for the array and to reflect the dynamic character put the array inside a struct with the last index, and when you add something add it on the last location and increment the counter. When this block is filled, you can chain another block to point to another one.

typedef struct _dyna_arr
{
  my_type data_arr[MAX_LEN];
  int n;
  struct _dyna_arr *next block;
};

Therefore you maintain a linked list of multiple arrays. The size of MAX_LEN can be fixed which is appropriate for an application which will help decrease internal fragmentation.

*old answer removed *

2 Comments

This is not what i'm trying to achieve. I don't want 2d array, I need an array of pointers, where every pointer is pointing to just one integer (or structure).
@Kijan: i understood that, and removed the old answer, i think the chained implementation would be much easier to implement and handle.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.