Dynamically allocating an array in a function in C

Question

I am having a problem in a program I am writing. This small program is a short version just to show the problem.

For this example, I am defining a structure called point which has an X and Y. I want the function to calculate the number of points, In this example I am assuming always 5, but this is not constant in my real program.

#include <stdio.h>

typedef struct  point {
   int x;
   int y;
}point;


// suppose this is dynamic. it return a value according to some parameter;
int howManyPoints() {
   // for this demo assume 5.
   return 5;
}


int createAnArrayOfPoints(point** outArray,int* totalPoints) {

   // how many points?
   int neededPoints = howManyPoints();

   // create an array of pointers
   *outArray =malloc(neededPoints * sizeof(point*));

   // malloc memory of a point size for each pointer
   for (int i=0;i<neededPoints;i++) outArray[i] = malloc(sizeof(point));

   // fill the points with some data for testing
   for (int k=0;k<neededPoints;k++) {
      outArray[k]->x = k*10;
      outArray[k]->y = k*5;
   }

   // tell the caller the size of the array
   *totalPoints = neededPoints;

   return 1;
  }


int main(int argc, const char * argv[]) {

   printf("Program Started\n");

   point* arrayOfPoints;
   int totalPoints;
   createAnArrayOfPoints(&arrayOfPoints,&totalPoints);

   for (int j=0;j<totalPoints;j++) {
    printf("point #%d is at %d,%d\n",j,arrayOfPoints[j].x,arrayOfPoints[j].y);
   }

   printf("Program Ended\n");

   return 0;
}

My console output looks like this:

Program Started
point #0 is at 0,0
point #1 is at 0,0
point #2 is at 10,5
point #3 is at 0,0
point #4 is at 20,10
Program Ended

What am I doing wrong? I am expecting all 5 points to have values in them..

Thanks.

In order to allocate an array of `point`, you only need a single `malloc()` call. It looks like you are allocating an array of `point*` instead. — Ulrich Eckhardt, Feb 22 '15 at 11:27
Your mistake is obvious already here: `*outArray =malloc(neededPoints * sizeof(point*));` the type which's size is taken to allocate is always one level less indirected as the type of the variable the memory should be allocated to. So as `*outArray` is `point*` the size to be take to allocate should have been the size of `point` but you use `point*`. — alk, Feb 22 '15 at 11:48

score 3 · Accepted Answer · answered Feb 22 '15 at 11:28

You have a mismatch in your representation for the array: In your main you are expecting an array of points (point* arrayOfPoints;) that is one consecutive piece of memory. However, the way you allocate it in createAnArrayOfPoints is different:

In that function you let arrayOfPoints point at a piece of memory only carrying pointers to point and initialize it with pointers to memory of the size of point you allocated. This is one indirection too much and also yields accesses outside of the allocated memory when printing.

Instead you should have done something like this:

// Allocate enough memory to store an array of points of the expected size.
// To keep the code more readable, the resulting pointer is stored in a 
// intermediate variable.
points *theArray = malloc(neededPoints * sizeof(point));
if (theArray == NULL) {
    // do some error handling here as you apparently ran out of memory...
}

// fill the points with some data for testing
for (int k=0;k<neededPoints;k++) {
   theArray[k].x = k*10;
   theArray[k].y = k*5;
}

// Now save the pointer to the output-variable.
*outArray = theArray;

Let me also add a word of warning: You always should check whether the malloc was successful or not before using the return value. It might be that you ran out of memory and therefor won't get what you requested.

VAndrei · Answer 2 · 2015-02-22T11:36:13.583

You don't need to use 2 mallocs. Try the code below. Your approach tries to actually create a matrix of points and initializes the first element of each line.

When you print you actually print the first line that hasn't been initialized fully.

int createAnArrayOfPoints(point** outArray,int* totalPoints) {

   // how many points?
   int neededPoints = howManyPoints();

   // create an array of pointers
   *outArray =(point*)malloc(neededPoints * sizeof(point));

   // fill the points with some data for testing
   for (int k=0;k<neededPoints;k++) {
      (*outArray)[k].x = k*10;
      (*outArray)[k].y = k*5;
   }

   // tell the caller the size of the array
   *totalPoints = neededPoints;

   return 1;
  }

score 0 · Answer 3 · answered Feb 22 '15 at 12:02

You should allocate an array of point structures, not an array of pointers to point structures. Furthermore, you are mixing indirection levels.

Use a local variable point *array; to hold the pointer to the allocated array and access it as array[i].x = k*10;... and store this pointer as *outArray = array

Dynamically allocating an array in a function in C

3 Answers3

Linked