problem with malloc

[christodoulos14]

hello i have some a problem with malloc, here is my code:

int add(int m,int n)
    {
    int **pina, **pinb, **pinc;
    int c=m*n, i=0, j=0;

    pina=NULL;
    pinb=NULL;
    pinc=NULL;

    free(pina);
    free(pinb);
    free(pinc);

    pina = (int **) malloc (sizeof(int *) *c);
    pinb = (int **) malloc (c* sizeof(int *));
    pinc = (int **) malloc (c* sizeof(int *));



    for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            pina[i][j]= NULL;
            pinb[i][j]= NULL;
            pinc[i][j]= NULL;
            }
        }


    printf("Give elements for A\n");

    for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            scanf("%d",&pina[i][j]);
            }
        }

    printf("Give elements for B\n");



    for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            scanf("%d",&pinb[i][j]);
            }
        }

      printf("\n");
    for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            printf("%d ",pina[i][j]);
            }
        printf("\n");
        } 

    printf("\n");
    for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            printf("%d ",pinb[i][j]);
            }
        printf("\n");
        } 

    free(pina);
    free(pinb);
    free(pinc);


    }

I just scanf array A and Array B from keyboart and then i print them.
the problem is that when i print array B is the same one with A.
I think tha somehow malloc doesn't work.
Any help plz?

[Gonbe]

This is incorrect:

pina = (int **) malloc (sizeof(int *) *c);
    pinb = (int **) malloc (c* sizeof(int *));
    pinc = (int **) malloc (c* sizeof(int *));

'c' is 'm' times 'n' so you're dynamically allocating an array of m*n pointers to integers. You're never allocating the things these should point to though. You can do something like this, but then you'll want to use a 1-dimensional array to store a 2-dimensional array.

So you have to change that to:

// Allocate the pointers to arrays
    pina = (int **) malloc (m * sizeof(int *));
    pinb = (int **) malloc (m * sizeof(int *));
    pinc = (int **) malloc (m * sizeof(int *));

And then add the following after it:

// Allocate the arrays.
    for (i = 0; i < m; i++)
    {
        pina[i] = (int*) malloc (n * sizeof(int));
        pinb[i] = (int*) malloc (n * sizeof(int));
        pinc[i] = (int*) malloc (n * sizeof(int));
    }

That's the direct cause of your problem, but you should remove this from the start:

free(pina);
    free(pinb);
    free(pinc);

and also this

for (i=0;i<m;i++)
        {
        for (j=0;j<n;j++)
            {
            pina[i][j]= NULL;
            pinb[i][j]= NULL;
            pinc[i][j]= NULL;
            }
        }

And freeing is changed to from this:

free(pina);
    free(pinb);
    free(pinc);

to this:

// Free the arrays.
    for (i = 0; i < m; i++)
    {
        free(pina[i]);
        free(pinb[i]);
        free(pinc[i]);
    }

    // Free the pointer to arrays.
    free(pina);
    free(pinb);
    free(pinc);

[deceptikon]

The following is completely unnecessary:

pina=NULL;
pinb=NULL;
pinc=NULL;

free(pina);
free(pinb);
free(pinc);

Calling free() on a null pointer is a no-op, and because you immediately call malloc() for each of those pointers, there's really no purpose in setting them to NULL to begin with.

A glaring problem is that you allocate essentially a 1D array, but then treat it as if it were a 2D array. That's a big no-no, and it's almost as if you're mixing two different methods and breaking them both in the process. Your first initialization loop should look more like this to properly simulate a 2D array (error checking omitted for brevity):

int **pina, **pinb, **pinc;
int i, j;

pina = (int**)malloc(m * sizeof(int*));
pinb = (int**)malloc(m * sizeof(int*));
pinc = (int**)malloc(m * sizeof(int*));

for (i = 0; i < m; i++) {
    pina[i] = (int*)malloc(n * sizeof(int));
    pinb[i] = (int*)malloc(n * sizeof(int));
    pinc[i] = (int*)malloc(n * sizeof(int));
}

Also don't forget to free the inner dimensions. All in all, a fixed add() would look something like this:

void add(int m, int n)
{
    int **pina, **pinb;
    int i, j;

    pina = (int**)malloc(m * sizeof(int*));
    pinb = (int**)malloc(m * sizeof(int*));

    for (i = 0; i < m; i++) {
        pina[i] = (int*)malloc(n * sizeof(int));
        pinb[i] = (int*)malloc(n * sizeof(int));
    }

    printf("Give elements for A: ");
    fflush(stdout);

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            scanf("%d", &pina[i][j]);
    }

    printf("Give elements for B: ");
    fflush(stdout);

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            scanf("%d", &pinb[i][j]);
    }

    printf("\n");

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            printf("%d ", pina[i][j]);
        printf("\n");
    } 

    printf("\n");

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            printf("%d ", pinb[i][j]);
        printf("\n");
    } 

    for (i = 0; i < m; i++) {
        free(pina[i]);
        free(pinb[i]);
    }

    free(pina);
    free(pinb);
}

But you have a bit of duplication, so it might be a good idea to defer the population and display to separate functions:

void populate(int **a, int m, int n)
{
    int i, j;

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            scanf("%d", &a[i][j]);
    }
}

void display(int **a, int m, int n)
{
    int i, j;

    for (i = 0; i < m; i++) {
        for (j = 0; j < n; j++)
            printf("%d ", a[i][j]);
        printf("\n");
    } 
}

void add(int m, int n)
{
    int **pina, **pinb;
    int i;

    pina = (int**)malloc(m * sizeof(int*));
    pinb = (int**)malloc(m * sizeof(int*));

    for (i = 0; i < m; i++) {
        pina[i] = (int*)malloc(n * sizeof(int));
        pinb[i] = (int*)malloc(n * sizeof(int));
    }

    printf("Give elements for A: ");
    fflush(stdout);
    populate(pina, m, n);

    printf("Give elements for B: ");
    fflush(stdout);
    populate(pinb, m, n);

    printf("\n");
    display(pina, m, n);

    printf("\n");
    display(pinb, m, n);

    for (i = 0; i < m; i++) {
        free(pina[i]);
        free(pinb[i]);
    }

    free(pina);
    free(pinb);
}

[christodoulos14]

thnks very much.
Very helpfull.