Skip to content

Commit 24364e2

Browse files
mstdokumacijinzhongjia
authored andcommitted
chore: apply suggested fixes
1 parent 948a04a commit 24364e2

2 files changed

Lines changed: 14 additions & 11 deletions

File tree

src/msgpack.zig

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,8 @@ fn clonePayload(payload: Payload, allocator: Allocator) !Payload {
868868
var cloned_count: usize = 0;
869869
errdefer {
870870
// cleanup partial clones on error
871-
var j: usize = cloned_count;
872-
while (j > 0) {
873-
j -= 1;
874-
new_arr[j].free(allocator);
871+
for (new_arr[0..cloned_count]) |item| {
872+
item.free(allocator);
875873
}
876874
allocator.free(new_arr);
877875
}
@@ -889,13 +887,18 @@ fn clonePayload(payload: Payload, allocator: Allocator) !Payload {
889887
// Clone all entries
890888
var it = m.map.iterator();
891889
while (it.next()) |entry| {
892-
const cloned_key = try clonePayload(entry.key_ptr.*, allocator);
893-
errdefer cloned_key.free(allocator);
894-
const cloned_value = try clonePayload(entry.value_ptr.*, allocator);
895-
errdefer cloned_value.free(allocator);
890+
var cloned_key: ?Payload = null;
891+
var cloned_value: ?Payload = null;
892+
errdefer {
893+
if (cloned_value) |v| v.free(allocator);
894+
if (cloned_key) |k| k.free(allocator);
895+
}
896+
897+
cloned_key = try clonePayload(entry.key_ptr.*, allocator);
898+
cloned_value = try clonePayload(entry.value_ptr.*, allocator);
896899

897900
// Use putInternal to insert without additional cloning
898-
try new_map.putInternal(cloned_key, cloned_value);
901+
try new_map.putInternal(cloned_key.?, cloned_value.?);
899902
}
900903
return Payload{ .map = new_map };
901904
},

src/test.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3618,7 +3618,7 @@ test "clonePayload arr partial fail path frees partially cloned elements" {
36183618
defer src.free(std.heap.page_allocator);
36193619

36203620
try src.setArrElement(0, try Payload.strToPayload("a", std.heap.page_allocator));
3621-
try src.setArrElement(1, try Payload.strToPayload("this-is-a-large-string-that-will-always-fail", std.heap.page_allocator));
3621+
try src.setArrElement(1, try Payload.strToPayload("this-is-a-very-large-string-that-will-always-fail-because-it-is-much-longer-than-the-fixed-buffer-can-handle-and-should-cause-out-of-memory-error-during-cloning-process", std.heap.page_allocator));
36223622

36233623
var buffer: [@sizeOf(Payload) * 2 + 8]u8 = undefined;
36243624
var pool = std.heap.FixedBufferAllocator.init(&buffer);
@@ -3641,7 +3641,7 @@ test "clonePayload map partial fail path frees partially cloned entries" {
36413641
defer src.free(std.heap.page_allocator);
36423642

36433643
try src.mapPut("k1", try Payload.strToPayload("v1", std.heap.page_allocator));
3644-
try src.mapPut("k2", try Payload.strToPayload("very-long-string-to-force-out-of-memory-on-clone", std.heap.page_allocator));
3644+
try src.mapPut("k2", try Payload.strToPayload("very-long-string-to-force-out-of-memory-on-clone-because-the-fixed-buffer-is-too-small-to-allocate-this-large-string-during-the-cloning-operation", std.heap.page_allocator));
36453645

36463646
var buffer: [@sizeOf(Payload) * 2 + 16]u8 = undefined;
36473647
var pool = std.heap.FixedBufferAllocator.init(&buffer);

0 commit comments

Comments
 (0)