Skip to content

Commit 224af46

Browse files
hyperpolymathclaude
andcommitted
fix(dialog): replace strlen walk in gossamer_dialog_free_path with std.c.free
The previous implementation walked bytes looking for the null terminator to reconstruct the allocation size, then passed that to Allocator.free. This was O(n), fragile (paths with embedded nulls would give wrong length), and technically incorrect — Zig's Allocator.free contract requires the same slice returned by alloc, which we cannot reconstruct accurately from a raw u64 pointer. Since c_allocator wraps libc malloc/free and free() needs no size, call std.c.free() directly. Add an explicit coupling comment warning maintainers that this function must stay paired with c_allocator. Add two unit tests: null-pointer no-op and round-trip allocate/free without crash. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2025cb7 commit 224af46

1 file changed

Lines changed: 30 additions & 6 deletions

File tree

src/interface/ffi/src/dialog.zig

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,37 @@ export fn gossamer_dialog_open_multiple(title: [*:0]const u8, filters: [*:0]cons
362362
/// Convenience function so callers do not need to know the allocator
363363
/// implementation. Safe to call with 0 (null).
364364
///
365+
/// Implementation note: path strings are allocated via `c_allocator` (libc
366+
/// malloc wrapped by `dupeZ`). At this call site we do not have the original
367+
/// slice length, so we call `std.c.free()` directly rather than routing
368+
/// through Zig's `Allocator.free` interface (which requires the original
369+
/// length to satisfy tracking-allocator contracts).
370+
///
371+
/// COUPLING: this function must remain paired with `c_allocator` in this file.
372+
/// If the allocator constant above is ever changed, this function must change
373+
/// to match — using the wrong free will corrupt the heap.
374+
///
365375
/// Matches: Gossamer.ABI.Foreign.prim__dialogFreePath
366376
export fn gossamer_dialog_free_path(path_ptr: u64) void {
367377
if (path_ptr == 0) return;
368-
const raw: [*]u8 = @ptrFromInt(@as(usize, @intCast(path_ptr)));
369-
// Find the null terminator to determine the length for free()
370-
var len: usize = 0;
371-
while (raw[len] != 0) : (len += 1) {}
372-
// Free the libc-allocated buffer (allocated via c_allocator / dupeZ)
373-
allocator.free(raw[0 .. len + 1]); // +1 to include the sentinel byte
378+
std.c.free(@ptrFromInt(@as(usize, @intCast(path_ptr))));
379+
}
380+
381+
//==============================================================================
382+
// Unit Tests
383+
//==============================================================================
384+
385+
test "gossamer_dialog_free_path: null pointer is a no-op" {
386+
// Calling with 0 must not crash or invoke free(NULL).
387+
gossamer_dialog_free_path(0);
388+
}
389+
390+
test "gossamer_dialog_free_path: frees c_allocator allocation without crash" {
391+
// Allocate a small buffer via c_allocator (the same allocator used by the
392+
// dialog functions) and verify that gossamer_dialog_free_path can free it.
393+
const buf = try allocator.dupeZ(u8, "/tmp/test-path");
394+
const ptr: u64 = @intCast(@intFromPtr(buf.ptr));
395+
// gossamer_dialog_free_path calls std.c.free — must not double-free or crash.
396+
gossamer_dialog_free_path(ptr);
397+
// buf is now freed; do NOT use it after this point.
374398
}

0 commit comments

Comments
 (0)