0

I have the following structure and initialisation of it in a C file:

struct Lattice {
    int length;
    int** latt;
};

struct Lattice* Create(int size){
    struct Lattice latt;
    int i,j,k,l;

    latt.length = size;
    latt.latt = malloc(sizeof(int*) * latt.length);

    for(i=0; i<latt.length; i++){
        latt.latt[i] = malloc(sizeof(int) * latt.length);
    }

        for(j=0; j<latt.length; j++){
            for(k=0; k<latt.length; k++){
                latt.latt[j][k] = 0;
            }
        }

    struct Lattice* lattp = &latt;
    return lattp;

And I also have the memory freeing function:

void Destroy(struct Lattice* lattp){
    int l;
    for (l=0; l<(lattp->length); l++){
        free(lattp->latt[l]);
    }
    free(lattp->latt);
}

But whenever I run the code:

int main(){
    struct Lattice* lattp = Create(5);
    Destroy(lattp);

    return 0;
}

I get a memory leak (according to Valgrind it is 40 bytes definitely lost and a further 80 indirectly lost, if this makes any difference).

But if I have identical code except that I write the free statements, which are identical to the ones in Destroy, into Create (and modify the return statements appropriately), I don't get a memory leak. Why does moving the free statements into a separate function cause a memory leak?

DanielJ
  • 73
  • 1
  • 7
  • 1
    Possible duplicate of [Can a local variable's memory be accessed outside its scope?](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – Sander De Dycker Jun 13 '18 at 13:37
  • 2
    That's because latt is a stack variable. You should not return the address of a stack variable from a function – Soumen Jun 13 '18 at 13:40
  • Because your create function doesn't allocate the structure. – Mad Physicist Jun 13 '18 at 13:41
  • 2
    In addition to the local variable issue, you shouldn't allocate 2D arrays this way. See [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). – Lundin Jun 13 '18 at 13:43
  • @SanderDeDycker My problem was that I didn't realise that I still have stuff in local scope (the length variable), so it isn't quite a duplicate I feel. Although for people not interested in this specific code it will probably be the same issue as the other post, not sure what the guidelines are for duplicated stuff in this case! – DanielJ Jun 13 '18 at 13:55
  • 1
    @DanielJ : me marking as duplicate does not mean you shouldn't have asked the question (you couldn't have known there was another question addressing the cause of your problem since you didn't know what the cause was). It's rather intended for future viewers of this question as you yourself indicated. Or to put it differently : in this case, the question isn't a duplicate, but the answer is. – Sander De Dycker Jun 13 '18 at 14:09
  • 1
    Unrelated to your problem, but instead of looping through every element and zeroing them, you can just use `calloc` instead of `malloc` when allocating space. – Christian Gibbons Jun 13 '18 at 14:14

1 Answers1

2

Your Create() function returns the address of a local variable:

struct Lattice* Create(int size){
    struct Lattice latt;  // this is a local variable - it only exists
                          // as long as the function Create() is running
    int i,j,k,l;

    latt.length = size;
    latt.latt = malloc(sizeof(int*) * latt.length);

    for(i=0; i<latt.length; i++){
        latt.latt[i] = malloc(sizeof(int) * latt.length);
    }

        for(j=0; j<latt.length; j++){
            for(k=0; k<latt.length; k++){
                latt.latt[j][k] = 0;
            }
        }

    struct Lattice* lattp = &latt;  // this takes the address of a local variable
    return lattp; // after this return, latt no longer exists
}

This is undefined behavior.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56