search

Arwalk / zig-protobuf

a protobuf 3 implementation for zig.
MIT License
219 stars 25 forks source link

discussion: Managed strings #18

Closed menduz closed 1 year ago

menduz commented 1 year ago

As of today, the library is leaking memory for user-allocated strings.

Tests are not failing because static/const strings are not accounted as leaks.

This could be fixed by recursively deallocating strings in the .deinit funciton but two problems may arise:

  1. It should be extremely clear for the user that the ownership of string memory is now passed onto the proto message. Plus, the allocator for the string MUST be exactly the same as the one used to deinitialize the message.
  2. Deallocating const strings may result in runtime exceptions

Proposal:

Replace the plain []const u8 by ManagedString. Keep the ?ManagedString for optional semantics. ManagedString is an union with OwnedString and ConstString, the former is automatically deallocated, the later won't.


const ManagedStringKeys = enum {
  OwnedString,
  ConstString
};

const ManagedString = union(ManagedStringKeys) {
  // memory that needs to be allocated
  OwnedString: []const u8,
  // memory that is static/const
  ConstString: []const u8,
};