Spot the bug: parsing a zon file

Challenge
1 
pub fn fromFile(io: std.Io, allocator: std.mem.Allocator, file_path: []const u8) !gcat.Arena(Config) {
    const arena = try allocator.create(std.heap.ArenaAllocator);
    errdefer allocator.destroy(arena);
    arena.* = .init(allocator);
    errdefer arena.deinit();
    const config_bytes = try std.Io.Dir.cwd().readFileAllocOptions(
        io,
        file_path,
        arena.allocator(),
        .unlimited,
        .@"1",
        0,
    );
    var diag: std.zon.parse.Diagnostics = .{};
    defer diag.deinit(allocator);

    const config = std.zon.parse.fromSliceAlloc(Config, arena.allocator(), config_bytes, &diag, .{}) catch |err| switch (err) {
        error.OutOfMemory => |e| return e,
        error.ParseZon => |e| {
            gcat.logger.err("failed to parse config file(zon): {f}", .{diag});
            return e;
        },
    };
    try config.validate();
    return gcat.Arena(Config){ .arena = arena, .value = config };
}
Hint

thread 120945 panic: Invalid free
And that’s why we use DebugAllocator folks!!

And as always, if you know a way to help prevent this type of bug, let me know!

1 Like
2

you pass allocator to diag.deinit but diagnostic allocations are probably done with the arena you pass to fromSliceAlloc?

One way to prevent that would be what std.json does, where the diagnostic is part of the reader/scanner, which you give a single allocator.

Another way would be for diagnostics to have its own allocator.

1 Like
3

Using the wrong allocator.

I’m also wondering why you’re heap-allocating the arena itself - aren’t those copyable within this kind of context?

As for preventing, simply ensuring this code has test coverage will suffice, but another thing might be to try and avoid this kind of code where you risk conflating the two allocators by doing the allocator creation in a higher place in the call tree than it’s use.