Free FixedBufferAllocator on loop

So I have this code that works

const std = @import("std");

fn walk(dir_path: []const u8) !void {
    var path_buff: [100]u8 = undefined;
    // var fba = std.heap.FixedBufferAllocator.init(&path_buff);
    // const allocator = fba.allocator();
    var dir = try std.fs.openDirAbsolute(
        dir_path,
        std.fs.Dir.OpenDirOptions{ .iterate = true },
    );
    defer dir.close();

    var dir_iter = dir.iterateAssumeFirstIteration();
    while (try dir_iter.next()) |current_dir| {
        // std.debug.print("entry: {}\n", .{std.json.fmt(current_dir, .{})});
        var fba = std.heap.FixedBufferAllocator.init(&path_buff);
        const allocator = fba.allocator();
        const path = try std.fs.path.joinZ(
            allocator,
            &.{ dir_path, current_dir.name },
        );
        std.debug.print("{s}\n", .{path});

        // Some logic with the path ...

        // This isnt freeing when the allocator declaration is outside the loop
        // allocator.free(path);
    }
}

pub fn main() !void {
    try walk("/bin");
}

But I imagine i would want the allocator declaration outside the loop. But when doing that, the free at the end doesnt seem to do anything and get errored out of space.
So how should i free it?

Also since im sharing this bit of code i might also ask 2 other smaller questions:
in

var dir = try std.fs.openDirAbsolute(
        dir_path,
        // Both options work
        std.fs.Dir.OpenDirOptions{ .iterate = true },
        { .iterate = true },
    );

The unnamed struct coerces to the needed struct. whats the consensus on how to write it? One is more explicit, the other less verbose

And the last, I belive FixedBufferAllocator is a really good option here but might as well ask if there are better.

I think this is a bit more correct and might be what you are looking for.

const std = @import("std");
const PATH_MAX = std.fs.max_path_bytes;

fn walk(dir_path: []const u8) !void {
    var path_buff: [PATH_MAX]u8 = undefined;
    var fba = std.heap.FixedBufferAllocator.init(&path_buff);
    const allocator = fba.allocator();
    var dir = try std.fs.openDirAbsolute(
        dir_path,
        std.fs.Dir.OpenDirOptions{ .iterate = true },
    );
    errdefer dir.close();

    var dir_iter = dir.iterateAssumeFirstIteration();
    while (try dir_iter.next()) |current_dir| {
        const path = try std.fs.path.joinZ(
            allocator,
            &.{ dir_path, current_dir.name },
        );
        std.debug.print("{s}\n", .{path});
        fba.reset();
    }
    dir.close();
}

pub fn main() !void {
    try walk("/bin");
}

So the FBA is just a bump allocator, what you need is not to free as much as reset it, such that it’s index inside it’s buffer goes back to zero on each loop.
The second little neat pick is for the size of the buffer, I’m not an expert on portability but I think using something akin to PATH_MAX (aka a numeric constant expressing what is the maximum number of bytes a path can be within a particular file system) is probably better than just 100 bytes.

6 Likes

Ah, I see! Thanks.
I also see the errdefer that was missing.
About the PATH_MAX, for what im doing even a 20 size buffer works,
but for generalization different filesystems have different limits, ext, ntfs, fat…
and that would be runtime known. But ofc setting the limit on what the OS specifies like max_path_bytes does is a good default.

1 Like

Actually i was transfering the solution to the actual code and it didnt work. There was a mistake on my part from the very start.

In the logic in handling the extracted path (which i didnt include in the example) to be able to have the free at the end of the loop i had to negate something that i missed to negate. So it wasnt that the free() wasnt working, it was that it was never getting there.

That means that both allocator.free(path); and fba.reset(); work
Im going with reset.

I would replace the errdefer with a normal defer and remove the dir.close() at the bottom.

3 Likes

but isn’t there a risk of getting an OOM ? within the loop (I mean it shouldn’t happen but maybe I’m not sure).

The defer always runs, the errdefer only runs when the scope is exited with an error, so using the defer makes sure the dir is always closed no matter why the scope is exited.

Usually you use errdefer when you only want to clean up things in the case of error (for example if the directory should stay open in the success case), but here the directory should always be closed after it was used.

1 Like

oh yes my bad, you are right thanks :wink:

1 Like