C: adding element to dynamically allocated array

Question

I've tried to search out a solution via Google: I couldn't find anything that helped; it even seemed as if I was doing this correctly. The only pages I could find regarding sending my dynamically allocated array through a function dealt with the array being inside a struct, which is scalar of course, so behaves differently. I don't want to use a struct right now -- I'm trying to learn about DAM and working with pointers and functions.

That said, I'm sure it's very elementary, but I'm stuck. The code compiles, but it freezes up when I run the executable. (I'm using minGW gcc, if that matters. And I'm not clear at all, right now, on how to use gdb.)

Here's the code (eventually, I want the entire code to be an ArrayList-like data structure):

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

void add( int element, int *vector);
void display_vector( int *vector );
void initialize_vector( int *vector );

int elements = 0;
int size = 10;

int main(void)
{
    int *vector = 0; 
    initialize_vector(vector);
    add(1, vector);
    //add(2, vector);
    //add(3, vector);
    //add(4, vector);
    //add(5, vector);
    //add(6, vector);
    //add(7, vector);
    //add(8, vector);
    //add(9, vector);
    //add(10, vector);
    //add(11, vector);
    display_vector(vector); 

    return 0;
}

void add( int element, int *vector)
{
    vector[elements++] = element;
    return;
}

void display_vector( int *vector )
{
    int i;
    for( i = 0; i < elements; i++)
    {
        printf("%2d\t", vector[i]);
        if( (i + 1) % 5 == 0 )
            printf("\n");
    }
    printf("\n");
    return; 
}

void initialize_vector( int *vector )
{
    vector = (int *)malloc(sizeof(int) * size);

}

Solution 1

Edited to make a little bit more clear.

The problem is your init routine is working with a copy of "vector" and is malloc'ing into that copy rather than the original vector pointer. You loose the pointer to the memory block on the return from the initialize.

Change parameter for vector to a handle (pointer to pointer) in this function

void initialize_vector( int **vector )
{
    *vector = (int *)malloc(sizeof(int) * size);
}

Then change the call to init to this

initialize_vector(&vector);

I didn't compile this, but it should fix the code.

Solution 2

In C, function arguments are passed by value, which means there is a local copy for every arguments you passed to a function, if you change an argument in a function, you only change the local copy of that argument. So if you want to change the value of an argument in a function, you need to pass its address to that function, derefer that address and assign to the result in that function.

Enough for the theory, here is how to fix your code:

void initialize_vector( int **vector );

initialize_vector(&vector);

void initialize_vector( int **vector )
{
    *vector = (int *)malloc(sizeof(int) * size);

}

Solution 3

In addition of other replies, I would suggest another approach.

Assuming at least C99 compliant compiler, I would rather suggest to keep the allocated size in a member of a structure ending with a flexible array member (see also this) like:

 typedef struct vector_st {
    unsigned count; // used length
    unsigned size;  // allocated size, always >= length
    int vectarr[];
 } Vector;

Then you would construct such a vector with

 Vector* make_vector (unsigned size) { 
   Vector* v = malloc(sizeof(Vector)+size*sizeof(int));
   if (!v) { perror("malloc vector"); exit (EXIT_FAILURE); };
   memset (v->vectarr, 0, size*sizeof(int));
   v->count = 0;
   v->size = size;
 }

To add an element into a vector, returning the original vector or a grown one:

Vector* append_vector (Vector*vec, int elem) {
  assert (vec != NULL);
  unsigned oldcount = vec->count;
  if (oldcount < vec->size) {
     vec->vectarr[vec->count++] = elem;
     return vec;
  } else {
     unsigned newsize = ((4*oldcount/3)|7) + 1;
     Vector* oldvec = vec;
     vec = malloc(sizeof(Vector)+newsize*sizeof(int));
     if (!vec) { perror("vector grow"); exit(EXIT_FAILURE); };
     memcpy (vec->vectarr, oldvec->vectarr, oldcount*sizeof(int));
     memset (vec->vectarr + oldcount, 0, 
             (newsize-oldcount) * sizeof(int));
     vec->vectarr[oldcount] = elem;
     vec->count = oldcount+1;
     vec->size = newsize;
     free (oldvec);
     return vec;
  }
}

and you could code:

Vector* myvec = make_vector(100);
myvec = append_vector(myvec, 35);
myvec = append_vector(myvec, 17);
for (int i=0; i<150; i++)
   myvec = append_vector(myvec, i*2);

To release such a vector, just use free(myvec);

---

If you really don't want to use any struct you should keep in separate variables the used length of your vector, the allocated size of your vector, the pointer to your dynamically allocated array:

  unsigned used_count; // useful "length"
  unsigned allocated_size; // allocated size, always not less than used_count
  int *dynamic_array; // the pointer to the dynamically allocated array

If you want to be able to manage several vectors, then either pack together the above useful length, allocated size and dynamic array into some struct dynamic_array_st (whose pointer you would pass to appropriate routines like make_dynamic_vector(struct dynamic_array_st*), append_dynamic_vector(struct dynamic_array_st*, int), etc ....) or else pass them as three separate formals to similar routines, and then you'll need to pass their address because the routines would change them, e.g. create_dynamic_vector(unsigned *countptr, unsigned *sizeptr, int**vectarrptr) that you would invoke as create_dynamic_vector(&mycount, &mysize, &myvectarr); etc.

I do think that a flexible array member is still the cleanest approach.

Comments

[Barmar]

It should be *vector = ...

[BWONG]

Thanks very much @Lee Duhem. I +1'd you.