Memory leak after using free() in destroyer function in c (according to splint)

Question

I am relearning C, and using splint to test my source code.

I am trying to do the following:

create a structure with a "constructor" function
destroy the structure with a "destructor" function, which frees the memory of the structure.

However, when I test my code with splint, it issues warnings related to temp storage within the destructor, and a memory leak after calling the destructor.

I am wondering (a) whether splint is correct about a memory leak in my code (I think it is not), and (b) what I should to do either fix my code or make splint understand what I am doing.

Anyway, here is the code:

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

// define the structure
struct Boring {
    int amount;
};

// the creator
static struct Boring *Boring_create(int amount) {
    struct Boring *really = malloc(sizeof(struct Boring));
    assert(really != NULL);
    really->amount=amount;
    return really;
}

// the destroyer
static void Boring_destroy(struct Boring *really) {
    assert( really != NULL );
    // free the memory of the Boring structure
    free(really);
}

int main( /*@unused@*/ int argc, /*@unused@*/ char *argv[]) {
    int amount = 5;
    struct Boring *tv = Boring_create(amount);
    printf("The TV is boring level %d\n",tv->amount);

    // destroy the tv!
    Boring_destroy(tv);

    printf("The TV is now boring level %d\n",tv->amount);
    return 0;
}
/* Output */
/*
* $> ./destroytv
* The TV is boring level 5
* The TV is now boring level -538976289 (or 0 depending on OS/compiler)
*/

The code compiles and runs fine with gcc.

However, when I use splint to test it, splint issues the following warnings:

$> splint boringtv.c
destroytv.c: (in function Boring_destroy)    
destroytv.c: Implicitly temp storage really passed as only param: free (really)
 Temp storage (associated with a formal parameter) is transferred to a new non-temporary reference. The storage may be released or new aliases crated. (Use -temptrans to inhibit warning)
destroytv.c: (in function main)
destroytv.c: Fresh storage tv not released before return
 A memory leak has been detcted. Storage allocated locally is not released before the last reference to it is lost (use -mustfreefresh to inhibit warning)
 Fresh storage tv created

The first warning, the more I think about it, the less I understand. But I haven't read enough of the manual to justify a proper question about that one.

The second warning, about the memory leak, it seems splint just doesn't realize that the memory is freed elsewhere, which seems strange to me. The warning goes away if I just call free() within main.

Thanks in advance for the help. Please let me know if more details like line numbers for the warnings would be helpful.

"Anyway, here is the gist of the code" - does the "gist" code posted here produce the actual messaging you've posted as well? And you're aware you're invoking undefined behavior with `printf("The TV is now boring level %d\n",tv->amount)`, right? — WhozCraig, Jun 26 '15 at 15:57
The code is copy pasted, the messaging is corrected to reflect the same code, minus line numbers. sorry for confusion, i removed gist. I didnt know thats undefined behavior. — hilcharge, Jun 26 '15 at 16:12
The `assert` creates a branch in your destructor which splint may not fully understand. What happens if you comment that out? — Useless, Jun 26 '15 at 16:29
My `splint` is a bit rusty, but I see that you have not annotated function `Boring_destroy()` to indicate that it takes exclusive ownership of its argument, which it must do to safely destroy it. If you add such an annotation, then `splint` should then complain about your dereference of `tv` in the `printf()` at the end of `main()`, which is a genuine error. — John Bollinger, Jun 26 '15 at 16:34
Also, never use `assert()` to perform any test that you are unwilling to do without. If an assertion ever fails, it should be a sign that the program is *wrong*, never merely that that the program has *failed*. The assertion in `Boring_destroy()` may meet those criteria, but the one in `Boring_create()` does not. — John Bollinger, Jun 26 '15 at 16:37
@JohnBollinger Thanks for the tip about taking ownership. It worked as you said, and I got the error for using tv after it had been freed. [This SO question](http://stackoverflow.com/questions/25351332/transfer-ownership-of-storage-in-splint) says how (it's using `/*@only@*/`). I'll add proper answer. For `assert`, interesting. I was following the "Learn C the Hard Way" tutorial, which uses it after `malloc` and in the destructor. Maybe he was just trying to introduce the concept eh. — hilcharge, Jun 27 '15 at 00:09

score 3 · Accepted Answer · edited May 23 '17 at 12:23

There is no memory leak, as suspected.

In order to tell splint what function should have control of the structure's memory, the input to the destructor function should be annotated with /*@only@*/.

static void Boring_destroy( /*@only@*/ struct Boring *really ) {...

This tells splint that that function is taking sole control of the variable, thus allowing it to free the memory peacefully without raising any warnings.

More specifically, the only annotation 'indicate[s] a reference is the only pointer to the object it points to.' (splint manual)

The annotation removes both the warnings mentioned in the original question, and replaces them with a warning indicating that tv is being used after it has been destroyed. This new warning is desirable, because as mentioned by WhozCraig in the comments, calling on memory once it has been freed is undefined behavior and hence should be avoided.

References:

SO questioin about splint and memory ownership
splint manual (See 5.2.1. Unshared References)

Memory leak after using free() in destroyer function in c (according to splint)

1 Answers1