[PATCH] Erase memory before freeing.

[Ben Kibbey]

On Mon, Aug 01, 2016 at 12:15:31PM +0200, Justus Winter wrote:
> Hi :)
> 
> Ben Kibbey <bjk at luxsci.net> writes:
> 
> > * src/mem.h: New.
> 
> Looks like you forgot to add this file.

Sure did. Oops. :)

> > +static struct mem_item_s *memlist;
> 
> I wonder if a list is the right data structure.  Linear lookup time
> feels expensive.

I thought this too. I haven't done any benchmarks but the list is
prepended to so it comforms with most de/allocations as LIFO. I also
don't khow how else to do it without requiring a realloc of a mem_item_s
which would corrupt pointers for .next. Any ideas?

> > +  p = malloc (n);
> > +  if (!p)
> > +    return NULL;
> > +
> > +  item = malloc (sizeof (struct mem_item_s));
> 
> I'd suggest to allocate both in one chunk.  That makes the error handling
> simpler, decreases pressure on the allocator and increases locality.

See above.

> > +  LOCK (memlist_lock);
> > +  item = find_memlist_item (p, 0);
> > +  if (!item)
> 
> I'm confused, shouldn't that be
> 
> if (p == NULL) return _gpgme_assuan_malloc (n);
> item = find_memlist_item (p, 0);
> assert (item);
> 
> instead ?

Yes. Thanks!

> 
> > +    {
> > +      UNLOCK (memlist_lock);
> > +      return _gpgme_assuan_malloc (n);
> > +    }
> > +
> > +  tmp = _gpgme_assuan_malloc (n);
> 
> Can't we just use realloc here?  And if not for some reason, please
> don't look newitem up like this, instead create a variant of
> _gpgme_assuan_malloc that returns the item.

Needed to wipe item->data. Not sure how to do both wiping item->data
and realloc'ing.

Attached is the revised patch. Suggestions welcome. Also, Werner
mentioned some time ago he'd like to add allocaters to gpg-error to
replace the "hooks" in libassuan and others. Not sure if anyone has
started that or not.

Thanks for the review.

-- 
Ben Kibbey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Erase-memory-before-freeing.patch
Type: text/x-diff
Size: 7658 bytes
Desc: not available
URL: </pipermail/attachments/20160801/b7badf77/attachment-0001.patch>