free() function causes crash after several memmove's

322 views Asked by At

I am writing a function acting like thesplice function in js:given an array (of any type), delete some element starting at a given index, and stuff some new element in the gap (expand or shirnk the original array if needed).

I am using MinGw/Eclipse CDT under Windows7. Here is my code:

void* splice(int typesize,void* arr,
        int size,int start, int length,
            void* stuff,int size2){
    //length is the number of elements to remove
    //and size2 is the number of elements to fill in the gap

    //so size-gap will be the size of the new array after the function
    //when gap is a minus number, the array grows
    //and when gap is a positive number, the array shrinks
    int gap = length-size2;
    void* ptr = malloc(typesize*(size-gap));//--------(1)--------
    if(ptr==NULL){
        puts("error");
        return NULL;
    }
    //now the ptr array is empty, copy the original array(arr)
    //to the ptr until the 'start' index
    memmove(ptr,arr,typesize*start);

    //fill the new array 'stuff' into ptr starting from 
    //the index after 'start'
    memmove(ptr+typesize*start,stuff,typesize*size2);

    //and copy the rest of the original array (starting from 
    //the index start+length, which means starting from 'start' index
    //and skip 'length' elements) into ptr
    memmove(ptr+typesize*(start+size2),arr+typesize*(start+length),
            typesize*(size-start-length));

    return ptr;
}

and I also write some test code, the snippet below is for long long type:

int main(){
    setbuf(stdout,NULL);
    int start = 1;
    int delete = 6;
    long long* oldArray= malloc(sizeof(long long)*7);
    long long* stuff = malloc(sizeof(long long)*3);
    oldArray[0]=7LL;
    oldArray[1]=8LL;
    oldArray[2]=4LL;
    oldArray[3]=1LL;
    oldArray[4]=55LL;
    oldArray[5]=67LL;
    oldArray[6]=71LL;
    stuff[0]=111LL;
    stuff[1]=233LL;
    stuff[2]=377LL;
    int newsize = 7-(delete-3);
    void* newArray = splice(sizeof(long long),oldArray,7,start,delete,stuff,3);
    if(newArray){

        //------------crash happens here-----------
        //free(oldArray);
        //-------------

        oldArray =  newArray;
        int i=0;
        for(;i<newsize;i++){
            printf("%I64d\n",oldArray[i]);
        }
    }
    return 0;
}

It should output 7, 111,233 and 377 (delete six elements from index 1 and stuff 111,233 and 377 into the array).

I tested for char, int, and long type arrays and under all situations the code worked. Except for one problem:I cannot free the old array. It seemes that the memory block cannot be reclaimed once it has been accessed several times by memmove.

If I change malloc to realloc at (1) and the free() won't crash, but I can no longer make the function work right (And I am not sure whether the free() function really worked or not).

Please give some advices about how this problem arise and how can I improve my code.

1

There are 1 answers

2
gnasher729 On BEST ANSWER

Look at this line:

    memmove(ptr,arr,typesize*size);

It tries to move typesize * size bytes to ptr. But you only allocated typesize*(size - gap) bytes. That will lead to a crash if gap > 0 unless you are very unlucky.

I stopped checking after the first bug I found, so there may be more, and I didn't bother finding out what the code does. You should add a comment which describes what the function should do well enough so that I could implement it without guessing or asking you questions.