PATCH: properly unset the zle registers hash

PATCH: properly unset the zle registers hash

[Oliver Kiddle]

Using the address sanitizer I noticed it leaking memory corresponding to
the hash table structure allocated for the zle registers. It seems the
zleunsetfn is not suitable for it and the hash table must be explicitly
deleted when the registers parameter goes out of scope. The registers
aren't internally in a hash table.

I'm not certain whether the stdunsetfn() call should be conditional on
(exp) like in zleunsetfn. Neither explicit nor implicit unsets are
clearing the registers so I assume this is fine.

Furthermore, it is necessary for the set function for hash variables to
check (ht != pm->u.hash) before deleting ht because a copy is used for
+= assignments. I noticed two other cases of this: compstate and
mapfile. Though, mapfile triggers the address sanitizer for an invalid
free before I can verify this path with it.

sourceforge git seems to be down by the way. I was about to push commits
but it'll have to wait now.

Oliver

diff --git a/Src/Modules/mapfile.c b/Src/Modules/mapfile.c
index 7a903418f..dd86fb596 100644
--- a/Src/Modules/mapfile.c
+++ b/Src/Modules/mapfile.c
@@ -158,7 +158,8 @@ setpmmapfiles(Param pm, HashTable ht)
 
 		setpmmapfile(v.pm, ztrdup(getstrvalue(&v)));
 	    }
-    deleteparamtable(ht);
+    if (ht != pm->u.hash)
+	deleteparamtable(ht);
 }
 
 /**/
diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 313dcb92f..1dc2b01c2 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -1325,7 +1325,8 @@ set_compstate(UNUSED(Param pm), HashTable ht)
 
 		    break;
 		}
-    deleteparamtable(ht);
+    if (ht != pm->u.hash)
+	deleteparamtable(ht);
 }
 
 /**/
diff --git a/Src/Zle/zle_params.c b/Src/Zle/zle_params.c
index 0a922d2d6..f3112165a 100644
--- a/Src/Zle/zle_params.c
+++ b/Src/Zle/zle_params.c
@@ -124,7 +124,7 @@ static const struct gsu_array killring_gsu =
 static const struct gsu_scalar register_gsu =
 { strgetfn, set_register, unset_register };
 static const struct gsu_hash registers_gsu =
-{ hashgetfn, set_registers, zleunsetfn };
+{ hashgetfn, set_registers, unset_registers };
 
 /* implementation is in zle_refresh.c */
 static const struct gsu_array region_highlight_gsu =
@@ -837,7 +837,17 @@ set_registers(UNUSED(Param pm), HashTable ht)
 
 	    set_register(v.pm, getstrvalue(&v));
         }
-    deleteparamtable(ht);
+    if (ht != pm->u.hash)
+	deleteparamtable(ht);
+}
+
+/**/
+static void
+unset_registers(Param pm, int exp)
+{
+    stdunsetfn(pm, exp);
+    deletehashtable(pm->u.hash);
+    pm->u.hash = NULL;
 }
 
 static void

Re: PATCH: properly unset the zle registers hash

[Sebastian Gniazdowski]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On 7 April 2018 at 03:53, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:

> Though, mapfile triggers the address sanitizer for an invalid
> free before I can verify this path with it.
>

mapfile has significant room for quality and feature improvements, but the
fact that metafication and unicode are in the way is discouraging. I mean,
to find character N=1000, one has to traverse buffer with mbtowc, counting
unicode characters. So one feature could be allowing to index by bytes,
with use case like: get last 1000 bytes from large log file, then process
as unicode string, extracting last 10 lines. Metafication is in the way,
getarg/getindex cannot be customized like getter and setter, so
byte-indexing is problematic.

-- 
Best regards,
Sebastian Gniazdowski