Did I release the heap memory well?

Hi all,

I am trying to write a small Zig code that can be called by C.
A Zig function (reverse in this case) should allocate a string in heap, and pass the pointer to the C caller
Another Zig function (free_string in this case) should free the heap memory when the C caller passes the pointer, which points to the heap memory space allocated by the reverse.

I tried different methods, then I came to this state as the code below, but I am not sure if I really achieved my goal or not (apart from the quality :slight_smile:)

const std = @import("std");

var gpa = std.heap.GeneralPurposeAllocator(.{}){};
var allocator = gpa.allocator();

pub fn main() void {
    const backwards = "backwards";
    const backwards_copy = allocator.dupe(u8, backwards) catch undefined;
    const reversed: *[]u8 = reverse(&backwards_copy, 9);
    std.debug.print("{s} reversed is {s}\n", .{ backwards, reversed.* });

    free_string(reversed);

    // this prints a garbage string - I guess because the reversed is pointing the address already released?
    std.debug.print("{s} reversed is {s}\n", .{ backwards, reversed.* });

    return;
}

export fn reverse(ptr: *const []u8, length: u32) callconv(.C) *[]u8 {
    var reversed_str = allocator.alloc(u8, length + 1) catch undefined;
    if (&reversed_str[0] == undefined) return undefined;

    var i: u8 = 0;
    while (i < length) : (i += 1) {
        reversed_str[length - i - 1] = ptr.*[i];
    }

    return &reversed_str;
}

export fn free_string(ptr: *[]u8) void {
    if (ptr == undefined) {
        return;
    }
    const p: *const []u8 = ptr;
    allocator.free(p[0..0]);
}

Any comment will be appreciated.

My comments:

  • *[]u8 should basically always be avoided. A slice is already a pointer type, so a pointer to a slice is a pointer to a pointer–you instead almost always just want []u8 or []const u8 (or in the case of C interop, [*]u8, [*:0]u8, etc)
  • No need to heap allocate backwards_copy. If you make reverse take a [*]const u8 you can just pass "backwards" into it directly
  • defer/errdefer is usually what you want to use for resource cleanup
  • Calling the deinit function of the GeneralPurposeAllocator will check for leaks for you, so you won’t need to guess about whether or not you’re leaking anything
  • Comparison against undefined is always a bug, that’s not what undefined is for in Zig

With that in mind, here’s an alternate version:

const std = @import("std");

var gpa = std.heap.GeneralPurposeAllocator(.{}){};
var global_allocator = gpa.allocator();

pub fn main() !void {
    // check for leaks
    defer std.debug.assert(gpa.deinit() == .ok);

    const backwards = "backwards";
    const reversed_ptr = reverse(backwards, backwards.len) orelse {
        std.debug.print("failed to allocate reversed string\n", .{});
        return error.OutOfMemory;
    };
    defer free_string(reversed_ptr);

    const reversed = std.mem.span(reversed_ptr);
    std.debug.print("{s} reversed is {s}\n", .{ backwards, reversed });

    // or, if using `reverse_zig`, the above could instead just look like:
    // const reversed = try reverse_zig(global_allocator, "backwards");
    // defer global_allocator.free(reversed);
    // std.debug.print("{s} reversed is {s}\n", .{ "backwards", reversed });
}

fn reverse_zig(allocator: std.mem.Allocator, input: []const u8) ![:0]u8 {
    // It seems like you want this to be a NUL-terminated string
    var reversed_str = try allocator.allocSentinel(u8, input.len, 0);
    var i: usize = 0;
    while (i < input.len) : (i += 1) {
        reversed_str[input.len - i - 1] = input[i];
    }
    return reversed_str;
}

/// returns `null` on allocation failure
export fn reverse(ptr: [*]const u8, length: u32) callconv(.C) ?[*:0]u8 {
    const slice = ptr[0..length];
    return reverse_zig(global_allocator, slice) catch return null;
}

export fn free_string(ptr: [*:0]u8) void {
    const slice = std.mem.span(ptr);
    global_allocator.free(slice);
}
7 Likes

It took some time to understand your reply as it is very comprehensive answer.

Actually, I was trying to rewrite the Dart FFI examples written in C (samples/ffi/structs/structs_library/structs.c at main · dart-lang/samples · GitHub) since I thought making the interop between Flutter and Zig might be an interesting use-case.

Thanks a lot!

2 Likes

If you haven’t already, check out the Zig pointer documentation (and the part on optional pointers). Something like char * in C can mean many different things (pointer to a single char, pointer to many chars, pointer to a null terminated sequence of chars, etc), while in Zig you’re able to more precisely encode that information into the pointer type itself.

EDIT: Also, I updated the code above to make reverse_zig take an Allocator as a parameter, which is the Zig convention for functions that need to heap allocate.

2 Likes

As I mentioned earlier, I asked this question to deliver an example for Dart-Zig interop.

This repo I have has some working examples similar to Google’s Dart-C FFI examples:

One may find it is useful if trying to do some native tasks within Flutter app in various platforms.

Still little more work remains as I would like to add the async example, which allow us to notify Dart process from Zig sub process (possibly via the native callable listener). Then bi-directional call can be easier.

Since I am new to Zig, the quality of code might not be great, so please leave comment here or in the issue board. I will keep maintain this repo for future versions of Dart and Zig.

Lastly, big thanks to @squeek502 and other Zig users.