On 05/02/12 16:57, Gert Doering wrote:
Hi,

On Sun, Feb 05, 2012 at 12:51:25PM +0100, Adriaan de Jong wrote:
Signed-off-by: Adriaan de Jong<dej...@fox-it.com>
---
  buffer.c |   29 ++++++++++-------------------
  1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/buffer.c b/buffer.c
index 2f8e4b8..c39bbcb 100644
--- a/buffer.c
+++ b/buffer.c
@@ -310,28 +310,19 @@ gc_malloc (size_t size, bool clear, struct gc_arena *a)
  #endif
  {
    void *ret;
-  if (a)
-    {
-      struct gc_entry *e;

NACK on that.  We discovered that there are use cases where "second-level"
callers pass NULL, seemingly on purpose to allocate global memory instead
of "in the GC".

main(), in openvpn.c:

           /* initialize environmental variable store */
           c.es = env_set_create (NULL);

... which calls into misc.c:

struct env_set *
env_set_create (struct gc_arena *gc)
{
   struct env_set *es;
   ALLOC_OBJ_CLEAR_GC (es, struct env_set, gc);
   es->list = NULL;
   es->gc = gc;
   return es;
}

... and boom, assert() fail.


Adriaan saw the crash ("my nice OpenBSD test run!!!") and promised to work
on this on the way back in the train.

But should this actually call gc_malloc() in this case? gc_malloc is for things which are supposed to be tackled by the garbage collector.

Things aren't too easy here - but I'd rather see us cleaning up this instead of making gc_malloc() vague in its purpose.

$ git grep env_set_create
init.c:  c->c2.es = env_set_create (NULL);
manage.c:  struct env_set *es = env_set_create (&gc);
misc.c:env_set_create (struct gc_arena *gc)
misc.h:struct env_set *env_set_create (struct gc_arena *gc);
multi.c:    es = env_set_create (&gc);
openvpn.c:        c.es = env_set_create (NULL);

There are two places where env_set_create() is called with NULL. Let's rather see if we can provide a gc_arena pointer those places instead. Or do something which is more clear in regards to memory allocation in env_set_create().


kind regards,

David Sommerseth

Reply via email to