unsafe free-ing of memory

[evpopov]

All calls to lfs_free() suffer from 2 issues:

• The if() clause that they are in does NOT ensure that the pointer that is about to be free-ed is non-null
• After the call to lfs_free(), the pointer that was just free-ed is not set to null

Reproducible by 2 consecutive calls to lfs_unmount() causing an attempt to free the cache pointers twice. I don't know about other heap implementations, but mine cannot guard against double free attempts and corrupts the heap in an instant.

Here's my solution for lfs_deinit() that covers 3 of the 4 calls to lfs_free. I did the same in lfs_file_rawclose() which is the only other place that lfs_free is called.

static int lfs_deinit(lfs_t *lfs) {
    // free allocated memory
    if (!lfs->cfg->read_buffer && lfs->rcache.buffer) {
        lfs_free(lfs->rcache.buffer);
		lfs->rcache.buffer = NULL;
    }

    if (!lfs->cfg->prog_buffer && lfs->pcache.buffer) {
        lfs_free(lfs->pcache.buffer);
		lfs->pcache.buffer = NULL;
    }

    if (!lfs->cfg->lookahead_buffer && lfs->free.buffer) {
        lfs_free(lfs->free.buffer);
		lfs->free.buffer = NULL;
    }

    return 0;
}

[BenBE]

lfs_free calls free which has the NULL check integrated*. Thus no check has to be done for NULL in advance.

BUT, and I agree with the suggestion here, after lfs_free has been called, the pointer should be cleared explicitly.

*I.e. every sane libc implementation has. If it does not, don't use that libc variant …

[evpopov]

@BenBE
Yes you are absolutely correct and my heap implementation is just fine when it receives a null.
I guess I'm just one of those programmers that feels safer with the non-null check.... my bad.

[BenBE]

There's nothing wrong with the additional check. It's just not necessary.

[evpopov]

I created #790 to address the issue. Closing this to keep things clean.