[PATCH] fixes leakage in g_strdup_vprintf() with ENABLE_MEM_CHECK



The leakage problem I originally noticed in g_strdup_vprintf() turns
out to have a pretty simple fix in gmem.c: it turns out that when
ENABLE_MEM_CHECK is set, memory is never actually freed!  (It looks
like this was accidental, not intentional.)

The attached patch fixes massive leakage when ENABLE_MEM_CHECK is set,
and also fixes an assumption that SIZEOF_LONG == 4.  I've also taken
the liberty of changing g_free to clobber freed memory with a non-zero
value to make access to freed memory more obvious.

Hope that helps,
--
Martin Pool

who pays any attention
to the syntax of things
will never wholly kiss you;
		      -- e. e. cummings




cd /opt/src/glib-1.2.1/
diff -c /opt/src/glib-1.2.1/gmem.orig.c /opt/src/glib-1.2.1/gmem.c
*** /opt/src/glib-1.2.1/gmem.orig.c	Mon Mar 29 21:58:51 1999
--- /opt/src/glib-1.2.1/gmem.c	Thu Apr 22 00:07:16 1999
***************
*** 49,57 ****
   *
   * The first, at offset -2*SIZEOF_LONG, is used only if
   * ENABLE_MEM_CHECK is set, and stores 0 after the memory has been
!  * allocated and 1 when it has been freed.  The second, at offset
!  * -SIZEOF_LONG, is used if either flag is set and stores the size of
!  * the block.
   *
   * The MEM_CHECK flag is checked when memory is realloc'd and free'd,
   * and it can be explicitly checked before using a block by calling
--- 49,57 ----
   *
   * The first, at offset -2*SIZEOF_LONG, is used only if
   * ENABLE_MEM_CHECK is set, and stores 0 after the memory has been
!  * allocated and 1 or greater when it has been freed.  The second, at
!  * offset -SIZEOF_LONG, is used if either flag is set and stores the
!  * size of the block, not including the debugging fields.
   *
   * The MEM_CHECK flag is checked when memory is realloc'd and free'd,
   * and it can be explicitly checked before using a block by calling
***************
*** 74,79 ****
--- 74,83 ----
  #define MAX_MEM_AREA  65536L
  #define MEM_AREA_SIZE 4L
  
+ /* If MEM_ENABLE_CHECK is set, g_free'd blocks are clobbered by this
+  * value to make bugs more obvious. */
+ #define GARBAGE_FILL	0xCC
+ 
  #if SIZEOF_VOID_P > SIZEOF_LONG
  #define MEM_ALIGN     SIZEOF_VOID_P
  #else
***************
*** 405,415 ****
  	g_warning ("freeing previously freed memory\n");
        *t += 1;
        mem = t;
        
-       memset ((guchar*) mem + 8, 0, size);
- #else /* ENABLE_MEM_CHECK */
        free (mem);
- #endif /* ENABLE_MEM_CHECK */
      }
  }
  
--- 409,419 ----
  	g_warning ("freeing previously freed memory\n");
        *t += 1;
        mem = t;
+ 
+       memset ((guchar*) mem + 2 * SIZEOF_LONG, GARBAGE_FILL, size);
+ #endif /* ENABLE_MEM_CHECK */
        
        free (mem);
      }
  }
  
***************
*** 456,462 ****
    t = (gulong*) ((guchar*) mem - SIZEOF_LONG - SIZEOF_LONG);
    
    if (*t >= 1)
!     g_warning ("mem: 0x%08x has been freed %lu times\n", (gulong) mem, *t);
  #endif /* ENABLE_MEM_CHECK */
  }
  
--- 460,466 ----
    t = (gulong*) ((guchar*) mem - SIZEOF_LONG - SIZEOF_LONG);
    
    if (*t >= 1)
!     g_warning ("mem: %p has been freed %lu times\n", mem, *t);
  #endif /* ENABLE_MEM_CHECK */
  }
  

Diff finished at Thu Apr 22 00:11:56
		      



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]