Freeing memory allocated in a loop

Help
9

There’s two insights here:

  1. You’re heap allocating both the names and the people. The caller needs a way to free both.

  2. Allocating lots of things in a loop leads to messy cleanup code.

I think @TheShinx317 got it right, but even with errdefer/defer doing a lot of work to make sure the frees happen in every code path, using them correctly is tricky as soon as your allocation pattern becomes any more complicated than “single allocations at the start of a scope”.

So. That’s what “good” memory management looks like in Zig.

Single large allocations in the start of scopes or functions that can be cleaned up with simple uses of defer/errdefer. Learning to design code that works like that can be pretty tricky, but it’s a very rewarding part of low level work IMO.

Compare the ArrayList-based implementation to something like this:

// zig 0.14.1
const std = @import("std");
const Allocator = std.mem.Allocator;

const NetworkPerson = struct {
    first_name: []const u8,
    last_name: []const u8,
};

const Person = struct {
    full_name: []const u8,
};

// Since both the names and the []Person slice need the same lifetime,
// it makes sense to group them in a struct
const People = struct {
    slice: []Person,
    name_buffer: []u8,

    // Returns the size of the smallest possible buffer that can contain
    // all the names of []NetworkPerson
    fn getNameBufferSize(
        network_peeps: []const NetworkPerson,
    ) usize {
        var buffer_size: usize = 0;
        for (network_peeps) |np| {
            // fmt.count returns the minimum buffer size required for fmt.print
            buffer_size += std.fmt.count(
                "{s} {s}",
                .{ np.first_name, np.last_name },
            );
        }
        return buffer_size;
    }

    pub fn initFromNetwork(
        allocator: std.mem.Allocator,
        network_peeps: []const NetworkPerson,
    ) !People {
        const content = try allocator.alloc(
            Person,
            network_peeps.len,
        );
        errdefer allocator.free(content);

        const name_buffer = try allocator.alloc(
            u8,
            getNameBufferSize(network_peeps),
        );

        // Now that we're done allocating, we can tell
        // the zig compiler to scream loudly if we have
        // unhandled errors past this point, so we don't
        // accidentally add more error paths that need
        // errdefer cleanup logic
        errdefer comptime unreachable;

        var index: usize = 0;
        for (network_peeps, content) |np, *p| {
            // bufPrint does two things:
            //    puts the printed string in the buffer
            //    returns a slice of the string
            p.full_name = std.fmt.bufPrint(
                name_buffer[index..],
                "{s} {s}",
                .{ np.first_name, np.last_name },
            ) catch |err| switch (err) {
                // getNameBufferSize makes sure the buffer can fit the names,
                // so the error case is impossible and we can tell the
                // compiler that
                error.NoSpaceLeft => unreachable,
            };

            index += p.full_name.len;
        }

        return .{ .name_buffer = name_buffer, .slice = content };
    }

    pub fn deinit(people: People, allocator: std.mem.Allocator) void {
        allocator.free(people.slice);
        allocator.free(people.name_buffer);
    }
};

test "generates people from first and last names" {
    const allocator = std.testing.allocator;
    const network_peeps: []const NetworkPerson = &.{
        .{ .first_name = "John", .last_name = "Lennon" },
        .{ .first_name = "Paul", .last_name = "McCartney" },
        .{ .first_name = "George", .last_name = "Harrison" },
        .{ .first_name = "Ringo", .last_name = "Starr" },
    };

    const people: People = try .initFromNetwork(
        allocator,
        network_peeps,
    );
    defer people.deinit(allocator);

    try std.testing.expectEqualStrings(
        people.slice[0].full_name,
        "John Lennon",
    );
    try std.testing.expectEqualStrings(
        people.slice[1].full_name,
        "Paul McCartney",
    );
    try std.testing.expectEqualStrings(
        people.slice[2].full_name,
        "George Harrison",
    );
    try std.testing.expectEqualStrings(
        people.slice[3].full_name,
        "Ringo Starr",
    );
}

  1. People encapsulates that you have two things that need freeing, and it has paired init and deinit functions. That way it’s easy for client code to see that you need to defer deinit to clean up.

  2. The error cleanup path in initFromNetwork is simple, since the complicated logic happens on a code path that isn’t allowed to fail, and this invariant is compiler-enforced.

  3. (Bonus) You’re allocating memory way less frequently in cases where network_peeps is large. Memory allocation is slow. Memory allocation can fail.

3 Likes