2

The code below is supposed to create two 2D arrays (posa and posb) that have contiguous memory locations, and then copy posa over posb using memmove function. Surprisingly, it works well for "small" array sizes (e.g.: size1=size2=100), but I get a segmentation fault (core dump) for larger sizes (e.g.: size1=size2=300). As I am using dynamic memory allocation, I presume this is not a stack overflow issue like here... Can somebody explain what I'm doing wrong ?

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

int ** Allocation_array_x2(int size1, int size2){
    int **ptr;
    int i;
    ptr = malloc(size1 * sizeof(*ptr));
    if(ptr == NULL)  {        
        fprintf(stderr,"Allocation error");
        exit(1);
    }
    ptr[0] = malloc(size1 * size2 * sizeof(**ptr));
    for(i=1 ; i < size1 ; i++){
        ptr[i]=ptr[0]+(i * size2);
        if( ptr[i] == NULL) {
            fprintf(stderr,"Allocation error");
            exit(1);
        }
    }
    return ptr;                         
}

void Free_array_x2(int **ptr){
    free(ptr);
    ptr=NULL;
}

int main(){
    int size1=300, size2=300;
    int **posa, **posb;
    posa=Allocation_array_x2(size1,size2);
    posb=Allocation_array_x2(size1,size2);

    /* array_filling */
    for( int j = 0 ; j<size1 ; j++ ) {
        for( int k = 0 ; k<size2 ; k++ ) {
            posa[j][k] = 2*j+3*k+1;     
            posb[j][k] = 1000;
        }
    }

    memmove(posb,posa,size1*size2*sizeof(int));

    for(int i = 0 ; i<size1 ; i++ ) {
        for(int j = 0 ; j<size2 ; j++ ) {
            printf("%d\t%d\n",posa[i][j],posb[i][j]);
        }
    }


    Free_array_x2(posa);
    Free_array_x2(posb);

}
Community
  • 1
  • 1
MarcPotts
  • 23
  • 3
  • where does segmentation happens exactly? – 4pie0 Sep 14 '15 at 19:37
  • The check for the second `malloc` must be on `ptr[0]`. If that is not `NULL`, all subsequent pointers are not `NULL`. And your `free` method should `free` both `ptr[0]` and `ptr`. – M Oehm Sep 14 '15 at 19:43
  • yes this is leaking memory, you are only making one free call for two mallocs – Chris Beck Sep 14 '15 at 19:43
  • If you are going to allocate all the required memory to `ptr[0]` and then parcel it out to the other array elements (in the interest of being contiguous), I feel it would be better to treat it as a 1-D array and manipulate the indices instead, using `row*size2+col`. – Weather Vane Sep 14 '15 at 19:47

2 Answers2

3

Changing the code to this fixed it for me:

memmove(*posb,*posa,size1*size2*sizeof(int));

The issue is that your main block of memory is actually at posa[0] and posb[0], not posa and posb. posa and posb just point to the secondary allocations.

Speaking of which, should point out that you are leaking memory -- your create function contains 2 malloc calls, but your free function only frees one of them.

Chris Beck
  • 15,614
  • 4
  • 51
  • 87
1

Error is here:

memmove(posb,posa,size1*size2*sizeof(int));

because you pass pointers to pointers to integer, but you shoud pass pointer to integer instead:

memmove(*posb, *posa, size1 * size2 * sizeof(int));

You have also errors in Free_array_x2 method because you don't release all memory you have taken from the system with malloc

void Free_array_x2(int **ptr){
    free(ptr);
    ptr=NULL;
}

This should go backward to what you do in allocation routine:

void Free_array_x2(int **ptr)
{
    free(ptr[0]);
    ptr[0] = NULL;
    free(ptr);
    ptr = NULL;
}
4pie0
  • 29,204
  • 9
  • 82
  • 118
  • Thanks !!! You're amazing guys ! This thing of pointers combined with array brackets [] notation is far beyond me I'm afraid. I cannot explain why it did work for small array sizes though ?! The mysteries of Memory Allocation... – MarcPotts Sep 14 '15 at 21:00