Memory leaks or not?
AssertionBit opened this issue · 2 comments
AssertionBit commented
Zig Version
0.12.0
Steps to Reproduce and Observed Behavior
No matter what to do here, passed allocator will cause neither not free memory or segmentation fault. Using errdefer
is causing mostly segmentation error, which means for me, that there is nothing I can do.
Use this code in any file:
const std = @import("std");
pub const CellPrimitiveType = enum(u8) {
Char = 0x1,
String = 0x2,
Integer = 0x3,
Long = 0x4,
Binary = 0x5,
ForeignIndex = 0x6,
};
pub const CellAttributes = enum(u8) {
Index = 0x1,
NotNull = 0x2,
Nullable = 0x3,
Unique = 0x4,
NonUnique = 0x5,
};
/// @brief Cell initialization error
///
/// Error appearing when creating new cell entity.
pub const CellInitError = error{
EmptyDataSet,
NameMissing,
TypeDefinitionMissing,
AttributeIsMissing,
AllocationResizeRefused,
};
/// @brief Collection of errors related to collisions
///
/// This set of errors contains all elements that possibly might collisions between values.
pub const CellCollisionError = error{
AttributeExists,
AttributePermissionError,
IndexExists,
};
/// @brief Single cell element
/// @author Mark Sorokin <assertionbit@icloud.com>
///
/// Cell is structure containging single element of table. It might be collected into row
/// where they will hold data or be in table, representing single data type.
pub const Cell = struct {
alloc: *const std.mem.Allocator,
cell_name: []u8,
cell_type: CellPrimitiveType,
cell_attributes: []CellAttributes,
value: []u8,
/// @brief Initiate cell with it's definition
/// @param alloc - Pointer to used allocator
/// @param data - Array of data which represents initial definition, without null-terminator
/// @return Cell definition element, without data
/// @throws Error{OutOfMemory} - When out of memory
/// @throws CellInitError - When provided data is not correct
pub fn init(alloc: *const std.mem.Allocator, data: []u8) !*const Cell {
// Data must not be null
if (data.len == 0) {
return CellInitError.EmptyDataSet;
}
// Name of cell
var cell_name: []u8 = try alloc.alloc(u8, data.len);
var needToBreak = false;
var name_size: usize = 0;
while ((name_size < data.len) and (!needToBreak)) : (name_size += 1) {
if (((data[name_size] == 0x0) or (data[name_size] <= 0x6))) {
needToBreak = true;
continue;
}
cell_name[name_size] = data[name_size];
}
if (!alloc.resize(cell_name, name_size - 1)) {
return CellInitError.AllocationResizeRefused;
}
if (name_size - 1 == 0) {
return CellInitError.NameMissing;
}
if (cell_name.len == data.len) {
alloc.free(cell_name);
return CellInitError.TypeDefinitionMissing;
}
const cell_type: CellPrimitiveType = @enumFromInt(data[cell_name.len + 1]);
var cell_attributes: []CellAttributes = try alloc.alloc(
CellAttributes,
cell_name.len - 1 - 1,
);
if (cell_attributes.len == 0) {
cell_attributes = try alloc.alloc(CellAttributes, 2);
cell_attributes[0] = CellAttributes.Nullable;
cell_attributes[1] = CellAttributes.NonUnique;
return &Cell{
.alloc = alloc,
.cell_name = cell_name,
.cell_type = cell_type,
.cell_attributes = cell_attributes,
.value = undefined,
};
}
for (data, (cell_name.len + 1)..) |value, index| {
cell_attributes[index - cell_attributes.len] = @enumFromInt(value);
}
return &Cell{
.alloc = alloc,
.cell_name = cell_name,
.cell_type = cell_type,
.cell_attributes = cell_attributes,
.value = undefined,
};
}
pub fn deinit(this: *const Cell) void {
this.alloc.free(this.cell_name);
this.alloc.free(this.cell_attributes);
if ((this.cell_name.len != 0)) {
this.alloc.free(this.value);
}
}
/// @brief Get byte definition of cell
/// @param this - Pointer to original cell element
/// @return Null-terminated byte array containging difinition of cell
/// @apiNote Return value must be deallocated
/// @throws Error{OutOfMemory} when machine reaches memory limit
///
/// Method returns byte definition of cell which might be used for writing
/// table definition. It helps internals to understand what is actual value
/// to expect.
pub fn getDefinition(this: *const Cell) ![]u8 {
var definition: []u8 = try this.alloc.alloc(u8, this.cell_name + 1 + this.cell_attributes.len + 1);
// Injecting name of value
for (this.cell_name, 0..) |char, index| {
definition[index] = char;
}
definition[this.cell_name.len + 1] = this.cell_type;
// Injecting attributes
for (this.cell_attributes, 0..) |value, index| {
definition[this.cell_name + 1 + index] = value;
}
// Null-terminator
definition[definition.len - 1] = 0x0;
return definition;
}
pub fn addAttribute(this: *const Cell) CellCollisionError!void {
_ = this;
}
};
test "Test empty data set" {
var dataset = [_]u8{};
const errorValue = Cell.init(&std.testing.allocator, &dataset);
try std.testing.expectError(CellInitError.EmptyDataSet, errorValue);
}
test "Testing failing name" {
var dataset = [_]u8{
0x0, 0x1, 0x2, 0x3,
};
const errorValue = Cell.init(&std.testing.allocator, &dataset);
try std.testing.expectError(CellInitError.NameMissing, errorValue);
}
test "Test missing attributes" {
var dataset = [_]u8{ 'E', 'E', 0x1 };
const result = try Cell.init(&std.testing.allocator, &dataset);
defer result.deinit();
}
test "Testing full cell" {
var dataset = [_]u8{ 'E', 'E', 0x1 };
const result = try Cell.init(&std.testing.allocator, &dataset);
defer result.deinit();
}
Expected Behavior
Using errdefer
in init
I may free memory, which was used during runtime. Either deinit
must free memory, but it doesnt' work neither.
AssertionBit commented
System info about this bug:
Darwin 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:12:41 PDT 2024; root:xnu-10063.101.17 arm64
squeek502 commented
return &Cell{
is returning a pointer to stack memory, which goes out-of-scope when the function returns. If you want to return a pointer toCell
, you'll need to heap allocate the memory for theCell
.- Side note: you shouldn't be passing/storing
std.mem.Allocator
as a pointer, you should accept/store astd.mem.Allocator
value (soCell.init(std.testing.allocator, &dataset)
andalloc: std.mem.Allocator,
)
In the future, ask for help in one of the community spaces before turning to the issue tracker. Closing this, please open a new issue if there ends up being a Zig bug involved here.