Step 3 of 6

Review: leaks on the error path

In C-style code, every early return is a chance to leak. Reviewers trace each exit path and check that everything acquired before it gets released.

Tracing exit paths

Go through the function top to bottom and keep a list of what's currently held. At every return, check the list:

values = xmalloc(...)      held: values
  return -1 (bad number)   still held: values        <- must free here
unique = xmalloc(...)      held: values, unique
  return -1 (n == 0)       still held: both          <- must free here
xfree(unique); xfree(values)
return n                   held: nothing             ok

The single cleanup label

C code commonly handles this with one exit point: every error sets a result and jumps to a label that frees everything. Freeing a null pointer does nothing, so it's safe even when some allocations never happened.

#include <cstdio>
#include <cstdlib>

int live = 0;
void* xmalloc(std::size_t n) { void* p = std::malloc(n); if (p) live++; return p; }
void xfree(void* p) { if (p) live--; std::free(p); }

int sum_positive(const int* in, int n) {
    int result = -1;
    int* a = nullptr;
    int* b = static_cast<int*>(xmalloc(sizeof(int) * n));
    if (!b) goto cleanup;
    a = static_cast<int*>(xmalloc(sizeof(int) * n));
    if (!a) goto cleanup;
    result = 0;
    for (int i = 0; i < n; i++) {
        if (in[i] < 0) { result = -1; goto cleanup; }   // error path: no leak
        a[i] = b[i] = in[i];
        result += a[i];
    }
cleanup:
    xfree(a);
    xfree(b);
    return result;
}

int main() {
    int good[] = {1, 2, 3}, bad[] = {1, -2, 3};
    int a = sum_positive(good, 3);
    int b = sum_positive(bad, 3);
    std::printf("%d %d, live allocations: %d\n", a, b, live);
}

6 -1, live allocations: 0

Every path, success or failure, ends at cleanup, so both blocks are always released. (In real C++ you'd use std::vector or std::unique_ptr and let destructors do this; this pull request stays in C style, which is common in code that talks to C libraries.)

Your turn: the happy path is fine, but some error paths leak. Make every path leave live_allocations where it started. There are two leaks. Staying in this C style is fine; a common tidy fix is a single cleanup label (goto cleanup;) or freeing before each return.

Previous: Review: dangling references Next: Review: modifying while iterating