Question

The code works if i use the name of the struct array directly for the allocation, but not from the function argument. Otherwise it returns memory error.

typedef struct COORD
{
  int xp;
  int yp;
} coord;

coord** xy;

void allocate(coord** COORD)
{
  int i;

  //allocate COORD[500][460]
  COORD = (coord**)malloc(sizeof(coord*)*500);
  for(i=0; i<500; i++)
  {
    COORD[i] = (coord*)malloc(sizeof(coord)*460);
  }

  // freeing
  for (i=0; i<500; i++) free(COORD[i]);
  free(COORD);
}
//function call: allocate(xy);
//That is the code that leeds to the error

Using just xy instead of COORD works. And i am all wondering why is that not working.

Was it helpful?

Solution

You are mixing up various coding styles here. It's not clear what exactly you want to achieve. Pick one according to your task.

Temporary buffer

You need a large temporary buffer that should be allocated on the heap and that does not need to be seen from outside. Just create a local variable:

void do_stuff(int w, int h)
{
    coord **p;
    int i;

    p = malloc(h * sizeof(*p));
    for (i = 0; i < h; i++) p[i] = malloc(w * sizeof(**p));;

    // do stuff

    for (i = 0; i < h; i++) free(p[i]);
    free(p);
}

Allocate memory for further use

You want to allocate storage that your client code can use. Then provide two functions, one that allocates and one that frees the memory:

coord **create(int w, int h)
{
    coord **p;
    int i;

    p = malloc(h * sizeof(*p));
    for (i = 0; i < h; i++) p[i] = malloc(w * sizeof(**p));

    return p;
}

void destroy(coord **p, int h)
{
    int i;

    for (i = 0; i < h; i++) free(p[i]);
    free(p);
}

Your client code can then use the memory between these calls:

coord **p = create(500, 460);

// do stuff

drestroy(p, 500);

(Note that you have to pass the height to destroy, which is a bit unfortunate. It might be cleaner to create a wrapper struct that hold information about width and height and the pointer.)

Allocate memory for a global variable

You have a single instance of a global pointer. Then your functions always operate on that pointer and you don't need any further information on it (except the dimensions):

coord **global = NULL;

void destroy_global(int h)
{
    int i;

    for (i = 0; i < h; i++) free(global[i]);
    free(global);
    global = NULL;
}  

void create_global(int w, int h)
{
    int i;

    if (global != NULL) free_global();

    global = alloc(h * sizeof(*global));
    for (i = 0; i < h; i++) global[i] = malloc(w * sizeof(**global));        
}

Note that you should include <stdlib.h> for all memory functions and the NULL macro.

Addendum According to your comment, you want to allocate memory for a bitmap. That's option 2 above.

I recommend to create an object structure. You can pass a pointerv to that structure as handle to a bunch of functions. You can create the object with a function that returns that handle.

The following sketches a rough design for a bitmap object.

typedef struct Pixel Pixel;
typedef struct Bitmap Bitmap;

struct Pixel {
    uint8_t r, g, b;
};

struct Bitmap {
    int height;
    int width;
    Pixel **pixel;
};

Bitmap *bitmap_new(int w, int h)
{
    Bitmap *bmp = malloc(sizeof(*bmp));
    int i;

    bmp->height = h;
    bmp->width = w;
    bmp->pixel = malloc(h * sizeof(*bmp->pixel));
    for (i = 0; i < h; i++) {
        bmp->pixel[i] = malloc(w * sizeof(**bmp->pixel));
    }

    return p;
}

void bitmap_delete(Bitmap *bmp)
{
    int i;

    for (i = 0; i < h; i++) free(bmp->pixel[i]);
    free(bmp->pixel);
    free(bmp);
}



Bitmap *bitmap_read(const char *fn)
{
    Bitmap *bmp;
    FILE *f = fopen(fn, "rb");

    // read and allocate 
    return bmp;
}

void bitmap_blank(Bitmap *bmp, int r, int g, int b)
{
    for (i = 0; i < bitmap->height; i++) {
        for (j = 0; j < bitmap->width; j++) {
            bmp->pixel[i][j].r = r;
            bmp->pixel[i][j].g = g;
            bmp->pixel[i][j].b = b;
        }
    }
}

void bitmap_mirror_x(Bitmap *bmp)
{
    // do stuff
}

int bitmap_write(Bitmap *bmp, const char *fn)
{
    FILE *f = fopen(fn, "rb");

    // write bitmap to file
    return 0;
}

The design is similar to the interface to FILE *: fopen gives you a handle (or NULL; error checking is omitted in the code above) and fread, fprintf, fseek and family take a pointer to the file as argument. Finally call fclose to close the file on disk and to free any ressources fopen has claimed.

OTHER TIPS

Have you tried to compile this code? There are a number of errors.

First, the type of main should always be 'int main(int argc, char *argv[])' Second, you need to '#include <stdlib.h>' at the top of your file to get the return type of malloc/free and friends. Third, you are not declaring 'i'. Fourth, you are using the same name 'COORD' as both a struct name and as a variable. Don't do this, it will cause you problems.

Sending incorrect code makes it very difficult to figure out what the root of your problem is, but I suspect it's the overloading of 'COORD'.

typedef struct COORD
{
  int xp;
  int yp;
} coord;

coord** xy;

void allocate(coord** COORD)
{
  int i;

  //allocate COORD[500][460]
  COORD = (coord**)malloc(sizeof(coord*)*500);
  for(i=0; i<500; i++)
  {
    COORD[i] = (coord*)malloc(sizeof(coord)*460);
  }

  // freeing
  for (i=0; i<500; i++) free(COORD[i]);
  free(COORD);
}
//function call: allocate();
//That is the code that works

The problem is that the function allocate() cannot change the value of xy outside itself. This is because C is call by value, the called function only gets the values of its arguments, not any kind of references to the expressions in the caller's context.

It needs to be:

void allocate(coord ***c)
{
}

and:

coord **xy;
allocate(&xy);

which of course is silly: the proper design would be for allocate() to return the new address:

coord ** allocate(void)
{
}

with use like:

coord **xy = allocate();

Probably it would be even better to have the dimensions as parameters to the function, since magic numbers are generally not a good thing:

coord ** allocate(size_t width, size_t height);
typedef struct
{
  int xp;
  int yp;
} Coord;

Coord **xy;

Coord** allocate(size_t height, size_t width)
{
  int i;
  Coord **arr;

  arr = malloc(sizeof(Coord*)*height);
  for(i=0; i<height; i++) {
    arr[i] = malloc(sizeof(coord)*width);
  }
  return arr;
}

void allocate2(Coord ***p_arr, size_t height, size_t width)
{
  int i;
  Coord **arr;

  arr = *p_arr;

  arr = malloc(sizeof(Coord*)*height);
  for(i=0; i<height; i++) {
    arr[i] = malloc(sizeof(coord)*width);
  }
}

void deallocate(Coord **arr, size_t height)
{
  for (i=0; i<500; i++) {
    free(arr[i]);
  }
  free(arr);
}

int main()
{
  Coord **arr_2;
  Coord ***p_arr_3;

  allocate2(&xy, 500, 460);
  /* do something with global array, xy, e.g. */
  xy[1][2].xp = 100;
  xy[1][2].yp = 200;
  deallocate(xy, 500);

  arr_2 = allocate(500, 460);
  /* do something with local array, arr_2 */
  deallocate(arr_2, 500);

  allocate2(p_arr_3, 500, 460);
  /* do something with ptr to local array, p_arr_3 */
  deallocate(*p_arr_3, 500);

  return 0;
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top