Clear content of dynamic memory inside structures from a function in C

Question

This is the sample version of my project. It throws a run-time error because I could not clear the contents and already allocated dynamic memory of data array in the bipartition_fn() function. Can someone please help me with clearing the contents of data in the bipartition structure after every reoccurrence of bipartition_fn() in the loop inside kdtree_fn()

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

typedef struct{
    int **data;
}kdtree;

typedef struct{
    int *data;
}bipartition;

void kdtree_fn(int *data, bipartition bp);
bipartition bipartition_fn(int *data);

int main(){
  int i;
  int *data;
  data = malloc(4*sizeof(int));
  for(i=0; i<4;i++)
    data[i] = i;
  bipartition bp;
  kdtree kd;
  kdtree_fn(data,bp);

}

void kdtree_fn(int *data, bipartition bp){
  kdtree k1;
  k1.data = malloc(5*4*sizeof(int));
  int i,j;
  for(j=0; j<5; j++){
    bp = bipartition_fn(data);
    for( i=0; i<4; i++){
      k1.data[j][i] = bp.data[i];
      printf("%d ",k1.data[j][i]);
    }
    printf("\n");
  }
  return k1;
}

bipartition bipartition_fn(int *data){
  bipartition bp1;
  int i;
  bp1.data = malloc(4*sizeof(int));
  for(i=0; i<4; i++){
    bp1.data[i] = data[i] +1;
  }
  return bp1;
}

`k1.data` is an `int**` type, you should change that to `k1.data = malloc(5*4*sizeof(int*));` And you need to allocate space for each of those `int*` types to point to before you do things like `k1.data[j][i] = ... ` — yano, Sep 08 '17 at 23:42
There is no array inside any of you structures. A pointer is not an array! — too honest for this site, Sep 09 '17 at 00:02
I was in a bit of a rush earlier.. really there's no point in dynamically allocating any of this. You know all the sizes at compile time, and the structures you want are "small" enough to fit into automatic storage (unless your system resources are terribly constrained). You should only dynamically allocate memory [when you really need to](https://stackoverflow.com/questions/1963780/when-should-i-use-malloc-in-c-and-when-dont-i) to free yourself from the task of managing memory manually. — yano, Sep 09 '17 at 06:14

Joshua Newhouse · Accepted Answer · 2017-09-10T03:46:35.807

As yano pointed out k1.data is of type pointer-to-pointer-to-int (int**) so you need to allocate first the array of int* and then allocate each array of int associated with each int*

I've made the changes to your code so that it now works without a runtime error. You look over it to see how k1.data is first allocated and then each element of k1.data is then allocated. Also note that I called free on everything that had previously been allocated.

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

typedef struct {
    int** data;
} kdtree;

typedef struct {
    int* data;
} bipartition;

void kdtree_fn(int*, bipartition);
bipartition bipartition_fn(int*);

#define ROW 5
#define COL 4

int main(void) {
    int data[COL] = {0};

    int i;
    for(i = 0; i < COL; i++)
        data[i] = i;

    bipartition bp;
    kdtree kd;
    kdtree_fn(data, bp);

}

void kdtree_fn(int* data, bipartition bp) {
    kdtree k1;
    k1.data = (int**)calloc(ROW, sizeof(int*));

    int i;
    for(i = 0; i < ROW; i++)
        k1.data[i] = (int*)calloc(COL, sizeof(int));

    int j;
    for(j = 0; j < ROW; j++) {
        bp = bipartition_fn(data);

        for(i = 0; i < COL; i++) {
            k1.data[j][i] = bp.data[i];
            printf("%d ",k1.data[j][i]);
        }
        printf("\n");

        free(bp.data);
    }

    for(i = 0; i < ROW; i++)
        free(k1.data[i]);
    free(k1.data);
}

bipartition bipartition_fn(int* data) {
    bipartition bp1;
    bp1.data = (int*)calloc(COL, sizeof(int));

    int i;
    for(i = 0; i < COL; i++)
        bp1.data[i] = data[i] + 1;

    return bp1;
}

I used calloc instead of malloc so that the memory is zeroed out rather than holding garbage values.

I changed your hard coded numbers (4 and 5) to symbolic constants ROW and COL so that you only need to change the numbers in a single place instead of throughout your code.

I made your variable 'data' an array instead of a dynamic array as there was no point in that.

It's worth mentioning that pointer-to-pointer types such as int** can be confusing. It's easier in my opinion to use a simple pointer type like int* and just access members using offset arithmetic. Instead of k1.data[j][i] with k1.data being type int** use (k1.data + j * ROW + i)* with k1.data being type int* — Joshua Newhouse, Sep 09 '17 at 00:52
Thank you for helping me but it did not solve my issue. If you closely observe the code I called the function bipartition_fn() in a loop. I don't want that to be removed. Because in my project each time you call the function it calculates something different. Can you please try by placing the function inside loop. — Moni, Sep 09 '17 at 23:58
The statement `bp = bipartition_fn(data);` in the kdtree_fn procedure is not changing due to the for loop in the code you have provided. It would be undesirable in this case to have it in the for loop as the result is always going to be the same. That being said, I've changed the above code that I provided so that that statement is now in the for loop. Hope it helps. — Joshua Newhouse, Sep 10 '17 at 03:45

cdlane · Answer 2 · 2017-09-09T07:03:34.437

My attempt at a rework of your code trying to keep all the pointers properly allocated and freed:

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

#define COLUMNS (4)
#define ROWS (5)

typedef struct {
    int **data;
} kdtree;

typedef struct {
    int *data;
} bipartition;

void bipartition_fn(int *data, bipartition *bp) {

    for (int i = 0; i < COLUMNS; i++) {
        bp->data[i] = data[i] + 1;
    }
}

void kdtree_fn(int *data, kdtree *kd) {

    bipartition bp;
    bp.data = calloc(COLUMNS, sizeof(int));

    for (int j = 0; j < ROWS; j++) {

        bipartition_fn(data, &bp);

        for (int i = 0; i < COLUMNS; i++) {
            kd->data[j][i] = bp.data[i];
            printf("%d ", kd->data[j][i]);
        }

        printf("\n");
    }

    free(bp.data);
}

int main() {

    kdtree kd;
    kd.data = calloc(ROWS, sizeof(int *));

    for (int j = 0; j < ROWS; j++) {
        kd.data[j] = calloc(COLUMNS, sizeof(int));
    }

    int *data = calloc(COLUMNS, sizeof(int));

    for (int i = 0; i < COLUMNS; i++) {
        data[i] = i;
    }

    kdtree_fn(data, &kd);

    // do something with kd

    for (int j = 0; j < ROWS; j++) {
        free(kd.data[j]);
    }

    free(kd.data);
    free(data);

    return 0;
}

I reordered the code and tossed the function prototypes to simplify the example and focus on memory mangement. Your original passes and returns structures which causes data to be copied -- I switched to passing around pointers to avoid the copying.

I want the memory allocation of the data variable to be done inside the function bipartition_fn() and still the code should work fine after looping it. Is that possible? — Moni, Sep 10 '17 at 00:00
@ManishaSiddartha, yes it's possible, but not desirable. When a function returns data, there's always a question of who's responsible for deallocating that data, if anyone. Particularly when there are two levels of deallocation (the struct and its data field). By putting responsibility on the caller to allocate the structure and pass it in, there's no question, from the caller's perspective, that they are responsible for deallocation. It's safer this way. However, if you look at my answer's edit history, you'll find my previous version works the way you describe and may be of use to you. — cdlane, Sep 10 '17 at 01:53

score -1 · Answer 3 · edited Sep 19 '17 at 04:43

-1

There are three ways to do so:

You can initialize an memory to zero using calloc function instead of malloc here you can read about it. Its also on the same header file and initialize the chunk memory allotted to zero.
other way is you can manually do it by bp->data[i]=0;
last way will be using bzero(char*,int) which zero all bits here is where you can read about it.

I prefer to use calloc because I don't have to call another function to initialize the memory thus little less time taking process.

edited Sep 19 '17 at 04:43

malioboro

3,097
4
35
55

answered Sep 09 '17 at 06:44

Ravinder Singh

69
10

`bzero` is obsolete and deprecated. Should be `memset(ptr, 0, bytesize)` which is standardized by C99 (in `` standard header). – Basile Starynkevitch Sep 09 '17 at 06:50
Also, `calloc` is not only clearing some memory, but most importantly *allocating* it (but `calloc` can fail, and calls to it should be checked). – Basile Starynkevitch Sep 09 '17 at 07:09

Clear content of dynamic memory inside structures from a function in C

3 Answers3