0

I already read these links: link1 and link2.

However, if I execute the following piece of code inside valgrind:

valgrind --tool=memcheck --leak-check=full --num-callers=40 --show-possibly-lost=no

I can see that the memory is not correctly freed.

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

void printVector(char ** vector, int N);
void allocateVector(char *** vector, int N, int M);
void deallocateVector(char *** vector, int N);

int main(int argc, char * argv[]) { 
    char ** vector;
    int N=6;
    int M=200;
    allocateVector(&vector,N,M);
    printVector(vector,N);
    deallocateVector(&vector,N);    
}

void allocateVector(char *** vector, int N, int M) {
    *vector=(char **) malloc(N*sizeof(char *));
    int i;
    for(i=0; i<N; i++) {
        (*vector)[i]=(char *) malloc(M*sizeof(char));
        (*vector)[i]="Empty";
    }
}

void deallocateVector(char *** vector, int N) {
    int i;
    char ** temp=*vector;
    for(i=0; i<N; i++) {
        if(temp[i]!=NULL) {
            free(temp[i]);
        }
    }
    if(temp!=NULL) {
        free(temp);
    }
    *vector=NULL;
}

I cannot find where is the mistake.

nedo
  • 11
  • 1
  • How can you see if the memory is not correctly freed? – Grantly Mar 08 '18 at 14:15
  • 3
    Please edit your question to include the full and unmodified output from Valgrind. And please point out locations in your code mentioned in the Valgrind output, but putting comments on those lines. – Some programmer dude Mar 08 '18 at 14:18
  • And please [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask), and learn how to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve). – Some programmer dude Mar 08 '18 at 14:19
  • 1
    `X = malloc(...); X = ...` is an immediate memory leak. – melpomene Mar 08 '18 at 14:20
  • Lastly, please find a good beginners book, or just about *any* beginners book. You make mistakes that a book should have learned you not to do. – Some programmer dude Mar 08 '18 at 14:21

1 Answers1

3

The problem is here:

for(i=0; i<N; i++) {
    (*vector)[i]=(char *) malloc(M*sizeof(char));
    (*vector)[i]="Empty";
}

You allocate space and store a pointer to it in (*vector)[i]. Then you overwrite that pointer with the address of the string constant "Empty".

This causes two issues:

  • The memory returned by malloc is leaked, as you no longer have a reference to it.
  • When you later call free, you're passing it the address of a string constant instead of the address of an allocated block of memory. Calling free in this way invokes undefined behavior.

You need to use the strcpy function to copy the string constant into the memory you allocated:

for(i=0; i<N; i++) {
    (*vector)[i]=malloc(M);
    strcpy((*vector)[i],"Empty");
}

Also, don't cast the return value of malloc, and sizeof(char) is defined to be 1 and can be omitted.

dbush
  • 205,898
  • 23
  • 218
  • 273