Re: [PATCH] cex: fix leaks

Hi Vincent,

> Le 17 mai 2020 à 15:59, Vincent Imbimbo <[email protected]> a écrit :
> 
> * src/state-item.c: Various functions were using heap allocated locals and not freeing them
> 
> Apple clang doesn't support leak sanitizer, so if this doesn't fix everything I'll bite the bullet and install a newer llvm. 

You definitely should install brew or MacPort's clang, using tools such as ASAN is life-changing.

Your patch improves things: we go from 

SUMMARY: AddressSanitizer: 262960 byte(s) leaked in 1874 allocation(s).

to

SUMMARY: AddressSanitizer: 187064 byte(s) leaked in 779 allocation(s).

The following commit reduces it further to

SUMMARY: AddressSanitizer: 184968 byte(s) leaked in 648 allocation(s).

I'm looking in what remains.

Cheers!

commit dcde6960d528c88d13bce1bd1de3590231c8b7f4
Author: Akim Demaille <[email protected]>
Date:   Sun May 17 16:28:08 2020 +0200

    cex: avoid gratuitous heap allocations
    
    There's no need to go for the heap when using gnulib's hash module.
    
    * src/state-item.c (hash_pair_lookup, hash_pair_remove,
    state_sym_lookup): Use the heap Luke.
    That removes a leak from hash_pair_lookup.
    (init_prods): Use hash_pair_insert instead of duplicating it.

diff --git a/src/state-item.c b/src/state-item.c
index 4a09e035..bcb21102 100644
--- a/src/state-item.c
+++ b/src/state-item.c
@@ -68,13 +68,10 @@ hash_pair_free (hash_pair *hp)
 static bitset
 hash_pair_lookup (Hash_table *tab, int key)
 {
-  hash_pair *l = xmalloc (sizeof (hash_pair));
-  l->key = key;
-  hash_pair *hp = (hash_pair *) hash_lookup (tab, l);
-  if (!hp)
-    return NULL;
-  free (l);
-  return hp->l;
+  hash_pair probe;
+  probe.key = key;
+  hash_pair *hp = hash_lookup (tab, &probe);
+  return hp ? hp->l : NULL;
 }
 
 static void
@@ -83,15 +80,16 @@ hash_pair_insert (Hash_table *tab, int key, bitset val)
   hash_pair *hp = xmalloc (sizeof (hash_pair));
   hp->key = key;
   hp->l = val;
-  hash_xinsert (tab, hp);
+  hash_pair *res = hash_xinsert (tab, hp);
+  assert (res == hp);
 }
 
 static void
 hash_pair_remove (Hash_table *tab, int key)
 {
-  hash_pair *hp = xmalloc (sizeof (hash_pair));
-  hp->key = key;
-  hash_delete (tab, hp);
+  hash_pair probe;
+  probe.key = key;
+  hash_delete (tab, &probe);
 }
 
 /* A state_item from a state's id and the offset of the item within
@@ -193,11 +191,9 @@ state_sym_comparator (const void *s1, const void *s2)
 static state *
 state_sym_lookup (symbol_number sym, Hash_table *h)
 {
-  state *s = xmalloc (sizeof (state));
-  s->accessing_symbol = sym;
-  state *res = hash_lookup (h, s);
-  free (s);
-  return res;
+  state probe;
+  probe.accessing_symbol = sym;
+  return hash_lookup (h, &probe);
 }
 
 static void
@@ -298,11 +294,8 @@ init_prods (void)
             {
               bitset copy = bitset_create (nstate_items, BITSET_SPARSE);
               bitset_copy (copy, lb);
-              hash_pair *prod_hp = xmalloc (sizeof (hash_pair));
-              prod_hp->key = j;
-              prod_hp->l = copy;
               // update prods.
-              hash_xinsert (si_prods, prod_hp);
+              hash_pair_insert (si_prods, j, copy);
 
               // update revs.
               bitset_iterator biter;