Skip to content

Commit 2360e35

Browse files
Merge pull request #40 from nix-community/feat/better-alloc
Fix memory leaks
2 parents dab3dc7 + c0e5d66 commit 2360e35

File tree

5 files changed

+83
-21
lines changed

5 files changed

+83
-21
lines changed

src/Dependency.zig

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1+
const std = @import("std");
2+
const Dependency = @This();
3+
14
url: []const u8,
25
rev: ?[]const u8,
36
nix_hash: ?[]const u8,
47
done: bool,
8+
9+
pub fn deinit(self: Dependency, alloc: std.mem.Allocator) void {
10+
alloc.free(self.url);
11+
if (self.rev) |rev| alloc.free(rev);
12+
if (self.nix_hash) |nix_hash| alloc.free(nix_hash);
13+
}

src/codegen.zig

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ pub fn write(alloc: Allocator, out: anytype, deps: StringHashMap(Dependency)) !v
1919
);
2020

2121
const len = deps.count();
22+
2223
var entries = try alloc.alloc(Entry, len);
24+
defer alloc.free(entries);
25+
2326
var iter = deps.iterator();
2427
for (0..len) |i| {
2528
entries[i] = iter.next().?;

src/fetch.zig

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const Worker = struct {
2727

2828
pub fn fetch(alloc: Allocator, deps: *StringHashMap(Dependency)) !void {
2929
var workers = try ArrayList(Worker).initCapacity(alloc, deps.count());
30+
defer workers.deinit();
3031
var done = false;
3132

3233
while (!done) {
@@ -45,12 +46,17 @@ pub fn fetch(alloc: Allocator, deps: *StringHashMap(Dependency)) !void {
4546
break :base try fmt.allocPrint(alloc, "tarball+{s}", .{dep.url});
4647
}
4748
};
49+
4850
const revi = mem.lastIndexOf(u8, base, "rev=") orelse break :ref base;
4951
const refi = mem.lastIndexOf(u8, base, "ref=") orelse break :ref base;
5052

53+
defer alloc.free(base);
54+
5155
const i = @min(revi, refi);
52-
break :ref base[0..(i - 1)];
56+
break :ref try alloc.dupe(u8, base[0..(i - 1)]);
5357
};
58+
defer alloc.free(ref);
59+
5460
log.debug("running \"nix flake prefetch --json --extra-experimental-features 'flakes nix-command' {s}\"", .{ref});
5561
const argv = &[_][]const u8{ nix, "flake", "prefetch", "--json", "--extra-experimental-features", "flakes nix-command", ref };
5662
child.* = ChildProcess.init(argv, alloc);
@@ -67,15 +73,18 @@ pub fn fetch(alloc: Allocator, deps: *StringHashMap(Dependency)) !void {
6773
const child = worker.child;
6874
const dep = worker.dep;
6975

76+
defer alloc.destroy(child);
77+
7078
const buf = try child.stdout.?.readToEndAlloc(alloc, std.math.maxInt(usize));
7179
defer alloc.free(buf);
7280

7381
log.debug("nix prefetch for \"{s}\" returned: {s}", .{ dep.url, buf });
7482

75-
var fbs = std.io.fixedBufferStream(buf);
76-
77-
var reader = json.reader(alloc, fbs.reader());
78-
const res = try json.parseFromTokenSourceLeaky(Prefetch, alloc, &reader, .{ .ignore_unknown_fields = true });
83+
const res = try json.parseFromSlice(Prefetch, alloc, buf, .{
84+
.ignore_unknown_fields = true,
85+
.allocate = .alloc_always,
86+
});
87+
defer res.deinit();
7988

8089
switch (try child.wait()) {
8190
.Exited => |code| if (code != 0) {
@@ -92,13 +101,15 @@ pub fn fetch(alloc: Allocator, deps: *StringHashMap(Dependency)) !void {
92101
},
93102
}
94103

95-
assert(res.hash.len != 0);
96-
log.debug("hash for \"{s}\" is {s}", .{ dep.url, res.hash });
104+
assert(res.value.hash.len != 0);
105+
log.debug("hash for \"{s}\" is {s}", .{ dep.url, res.value.hash });
97106

98-
dep.nix_hash = res.hash;
107+
dep.nix_hash = try alloc.dupe(u8, res.value.hash);
99108
dep.done = true;
100109

101-
const path = try fmt.allocPrint(alloc, "{s}" ++ fs.path.sep_str ++ "build.zig.zon", .{res.storePath});
110+
const path = try fmt.allocPrint(alloc, "{s}" ++ fs.path.sep_str ++ "build.zig.zon", .{res.value.storePath});
111+
defer alloc.free(path);
112+
102113
const file = fs.openFileAbsolute(path, .{}) catch |err| switch (err) {
103114
error.FileNotFound => continue,
104115
else => return err,

src/main.zig

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
const builtin = @import("builtin");
12
const std = @import("std");
23
const StringHashMap = std.StringHashMap;
34
const fs = std.fs;
@@ -10,6 +11,20 @@ const fetch = @import("fetch.zig").fetch;
1011
const parse = @import("parse.zig").parse;
1112
const write = @import("codegen.zig").write;
1213

14+
const zig_legacy_version = (std.SemanticVersion{
15+
.major = builtin.zig_version.major,
16+
.minor = builtin.zig_version.minor,
17+
.patch = builtin.zig_version.patch,
18+
}).order(.{
19+
.major = 0,
20+
.minor = 14,
21+
.patch = 0,
22+
}) == .lt;
23+
24+
const DebugAllocator = @field(std.heap, if (zig_legacy_version) "GeneralPurposeAllocator" else "DebugAllocator");
25+
26+
var debug_allocator: DebugAllocator(.{}) = if (zig_legacy_version) .{} else .init;
27+
1328
pub fn main() !void {
1429
var args = process.args();
1530
_ = args.skip();
@@ -24,16 +39,31 @@ pub fn main() !void {
2439
dir.openFile("build.zig.zon", .{});
2540
defer file.close();
2641

27-
var arena = heap.ArenaAllocator.init(heap.page_allocator);
28-
defer arena.deinit();
29-
const alloc = arena.allocator();
42+
const gpa, const is_debug = gpa: {
43+
break :gpa switch (builtin.mode) {
44+
.Debug, .ReleaseSafe => .{ debug_allocator.allocator(), true },
45+
.ReleaseFast, .ReleaseSmall => .{ std.heap.smp_allocator, false },
46+
};
47+
};
48+
defer if (is_debug) {
49+
_ = debug_allocator.deinit();
50+
};
51+
52+
var deps = StringHashMap(Dependency).init(gpa);
53+
defer {
54+
var iter = deps.iterator();
55+
while (iter.next()) |entry| {
56+
gpa.free(entry.key_ptr.*);
57+
entry.value_ptr.deinit(gpa);
58+
}
59+
deps.deinit();
60+
}
3061

31-
var deps = StringHashMap(Dependency).init(alloc);
32-
try parse(alloc, &deps, file);
33-
try fetch(alloc, &deps);
62+
try parse(gpa, &deps, file);
63+
try fetch(gpa, &deps);
3464

3565
var out = io.bufferedWriter(io.getStdOut().writer());
36-
try write(alloc, out.writer(), deps);
66+
try write(gpa, out.writer(), deps);
3767
try out.flush();
3868
}
3969

src/parse.zig

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,23 @@ const zig_legacy_version = (std.SemanticVersion{
2323

2424
pub fn parse(alloc: Allocator, deps: *StringHashMap(Dependency), file: File) !void {
2525
const content = try alloc.allocSentinel(u8, try file.getEndPos(), 0);
26+
defer alloc.free(content);
27+
2628
_ = try file.reader().readAll(content);
2729

28-
const ast = try Ast.parse(alloc, content, .zon);
30+
var ast = try Ast.parse(alloc, content, .zon);
31+
defer ast.deinit(alloc);
2932

3033
var root_buf: [2]Index = undefined;
3134
const root_init = ast.fullStructInit(&root_buf, @field(ast.nodes.items(.data)[0], if (zig_legacy_version) "lhs" else "node")) orelse {
3235
return error.ParseError;
3336
};
3437

3538
for (root_init.ast.fields) |field_idx| {
36-
if (!mem.eql(u8, try parseFieldName(alloc, ast, field_idx), "dependencies")) {
39+
const field_name = try parseFieldName(alloc, ast, field_idx);
40+
defer alloc.free(field_name);
41+
42+
if (!mem.eql(u8, field_name, "dependencies")) {
3743
continue;
3844
}
3945

@@ -61,18 +67,21 @@ pub fn parse(alloc: Allocator, deps: *StringHashMap(Dependency), file: File) !vo
6167

6268
for (dep_init.ast.fields) |dep_field_idx| {
6369
const name = try parseFieldName(alloc, ast, dep_field_idx);
70+
defer alloc.free(name);
6471

6572
if (mem.eql(u8, name, "url")) {
6673
const parsed_url = try parseString(alloc, ast, dep_field_idx);
6774
if (std.mem.startsWith(u8, parsed_url, "https://")) {
6875
url = parsed_url;
6976
} else if (std.mem.startsWith(u8, parsed_url, "git+https://")) {
77+
defer alloc.free(parsed_url);
78+
7079
const url_end = std.mem.indexOf(u8, parsed_url[0..], "#").?;
7180
const raw_url = parsed_url[4..url_end];
7281
const hash_start = url_end + 1; // +1 to skip the '#'
7382
const git_hash = parsed_url[hash_start..];
74-
url = raw_url;
75-
dep.rev = git_hash;
83+
url = try alloc.dupe(u8, raw_url);
84+
dep.rev = try alloc.dupe(u8, git_hash);
7685
}
7786
} else if (mem.eql(u8, name, "hash")) {
7887
hash = try parseString(alloc, ast, dep_field_idx);
@@ -91,7 +100,7 @@ pub fn parse(alloc: Allocator, deps: *StringHashMap(Dependency), file: File) !vo
91100

92101
fn parseFieldName(alloc: Allocator, ast: Ast, idx: Index) ![]const u8 {
93102
const name = ast.tokenSlice(ast.firstToken(idx) - 2);
94-
return if (name[0] == '@') string_literal.parseAlloc(alloc, name[1..]) else name;
103+
return if (name[0] == '@') string_literal.parseAlloc(alloc, name[1..]) else alloc.dupe(u8, name);
95104
}
96105

97106
fn parseString(alloc: Allocator, ast: Ast, idx: Index) ![]const u8 {

0 commit comments

Comments
 (0)