From 6a78b315b2112e372b48ba7399ea9cddadbe65b6 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Sat, 6 Apr 2019 14:15:12 -0700 Subject: [PATCH] Fix std.HashMap.remove returning incorrect KV Now returns a copy of the removed kv instead of a pointer to the removed kv. The removed kv gets overwritten when shifting the hash map after the removal, so returning a pointer to it will have another kv's values in it after the return. This bug had some nasty downstream effects in things like BufSet and BufMap where delete would free a still in-use KV and leave the actually removed KV un-free'd. --- std/hash_map.zig | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/std/hash_map.zig b/std/hash_map.zig index f4c0b87167..6ea128c9ad 100644 --- a/std/hash_map.zig +++ b/std/hash_map.zig @@ -175,7 +175,7 @@ pub fn HashMap(comptime K: type, comptime V: type, comptime hash: fn (key: K) u3 return hm.get(key) != null; } - pub fn remove(hm: *Self, key: K) ?*KV { + pub fn remove(hm: *Self, key: K) ?KV { if (hm.entries.len == 0) return null; hm.incrementModificationCount(); const start_index = hm.keyToIndex(key); @@ -189,13 +189,14 @@ pub fn HashMap(comptime K: type, comptime V: type, comptime hash: fn (key: K) u3 if (!eql(entry.kv.key, key)) continue; + const removed_kv = entry.kv; while (roll_over < hm.entries.len) : (roll_over += 1) { const next_index = (start_index + roll_over + 1) % hm.entries.len; const next_entry = &hm.entries[next_index]; if (!next_entry.used or next_entry.distance_from_start_index == 0) { entry.used = false; hm.size -= 1; - return &entry.kv; + return removed_kv; } entry.* = next_entry.*; entry.distance_from_start_index -= 1; @@ -371,7 +372,10 @@ test "basic hash map usage" { testing.expect(map.contains(2)); testing.expect(map.get(2).?.value == 22); - _ = map.remove(2); + + const rmv1 = map.remove(2); + testing.expect(rmv1.?.key == 2); + testing.expect(rmv1.?.value == 22); testing.expect(map.remove(2) == null); testing.expect(map.get(2) == null); }