add vips_tracked_malloc()

new malloc()/free() pair do tracked allocations ... use g_new()/g_free()
everywhere else

ChangeLog

@@ -12,7 +12,7 @@
 - fix up VipsPool
 - add the operation cache
 - fallback vips_init()
-- vips_malloc() tracks allocation size and can report total mem usage
+- vips_tracked_malloc() tracks allocation size and can report total mem usage
 
 10/8/11 started 7.26.3
 - don't use G_VALUE_COLLECT_INIT(), many platforms do not have a glib this

TODO

@@ -1,9 +1,10 @@
-- vips_malloc() now tracks alloc size, but we seem to g_free() the results
+- make pixel buffers etc. use new tracked malloc thing
-  sometimes, look into this
+
+  add LRU dsrop
+
+  add something to drop the whole cache for leak debugging
+
 
-  argh no too confusing ... make an im_malloc() that's just 
-  g_try_alloc()/g_free() with auto-free, keep vips_alloc() for new stuff, and
-  for buffers
 
 
 
@@ -13,13 +14,6 @@
 
 
 
-
-
-- cache work ... see notes in file
-
-
-
-
 - add vips_init_argv() which processes argc/argv for you? handy for tiny
   progs, perhaps
 

libvips/arithmetic/avg.c

@@ -143,8 +143,7 @@ vips_avg_start( VipsStatistic *statistic )
 {
 	double *sum;
 
-	if( !(sum = VIPS_NEW( NULL, double )) ) 
+	sum = g_new( double, 1 );
-		return( NULL );
 	*sum = 0.0;
 
 	return( (void *) sum );
@@ -160,7 +159,7 @@ vips_avg_stop( VipsStatistic *statistic, void *seq )
 
 	avg->sum += *sum;
 
-	vips_free( seq );
+	g_free( seq );
 
 	return( 0 );
 }

libvips/arithmetic/min.c

@@ -153,8 +153,7 @@ vips_min_start( VipsStatistic *statistic )
 	VipsMin *global = (VipsMin *) statistic;
 	VipsMin *min;
 
-	if( !(min = VIPS_NEW( NULL, VipsMin )) ) 
+	min = g_new( VipsMin, 1 );
-		return( NULL );
 	*min = *global;
 
 	return( (void *) min );
@@ -176,7 +175,7 @@ vips_min_stop( VipsStatistic *statistic, void *seq )
 		global->set = TRUE;
 	}
 
-	vips_free( min );
+	g_free( min );
 
 	return( 0 );
 }

libvips/colour/im_icc_transform.c

@@ -319,11 +319,8 @@ attach_profile( IMAGE *im, const char *filename )
 	if( !(data = im__file_read_name( filename, VIPS_ICC_DIR, 
 		&data_length )) ) 
 		return( -1 );
-	if( im_meta_set_blob( im, IM_META_ICC_NAME, 
+	im_meta_set_blob( im, IM_META_ICC_NAME, 
-		(im_callback_fn) im_free, data, data_length ) ) {
+		(im_callback_fn) im_free, data, data_length );
-		im_free( data );
-		return( -1 );
-	}
 
 	return( 0 );
 }

libvips/conversion/im_copy.c

@@ -251,9 +251,9 @@ im_copy( IMAGE *in, IMAGE *out )
 int
 im_copy_set_meta( IMAGE *in, IMAGE *out, const char *field, GValue *value )
 {
-	if( im_copy( in, out ) ||
+	if( im_copy( in, out ) )
-		im_meta_set( out, field, value ) )
 		return( 1 );
+	im_meta_set( out, field, value );
 
 	return( 0 );
 }

libvips/convolution/im_aconv.c

@@ -676,8 +676,7 @@ boxes_new( IMAGE *in, IMAGE *out, DOUBLEMASK *mask, int n_layers, int cluster )
 		vips_check_dmask( "im_aconv", mask ) ) 
 		return( NULL );
 
-	if( !(boxes = VIPS_NEW( out, Boxes )) )
+	boxes = VIPS_NEW( out, Boxes );
-		return( NULL );
 	boxes->in = in;
 	boxes->out = out;
 	if( !(boxes->mask = (DOUBLEMASK *) im_local( out, 

libvips/convolution/im_aconvsep.c

@@ -162,8 +162,7 @@ lines_new( IMAGE *in, IMAGE *out, DOUBLEMASK *mask, int n_layers )
 		vips_check_dmask_1d( "im_aconvsep", mask ) ) 
 		return( NULL );
 
-	if( !(lines = VIPS_NEW( out, Lines )) )
+	lines = VIPS_NEW( out, Lines );
-		return( NULL );
 	lines->in = in;
 	lines->out = out;
 	if( !(lines->mask = (DOUBLEMASK *) im_local( out, 

libvips/deprecated/dispatch_types.c

@@ -182,7 +182,7 @@ imagevec_dest( im_object obj )
 				iv->vec[i] = NULL;
 			}
 
-		vips_free( iv->vec );
+		g_free( iv->vec );
 		iv->vec = NULL;
 		iv->n = 0;
 	}
@@ -244,7 +244,7 @@ mask_init( im_object *obj, char *str )
 
 	/* Install string, clear mask.
 	 */
-	if( str && !(mo->name = vips_strdup( NULL, str )) ) 
+	if( str && !(mo->name = im_strdup( NULL, str )) ) 
 		return( -1 );
 	mo->mask = NULL;
 
@@ -288,14 +288,8 @@ dmask_dest( im_object obj )
 {
 	im_mask_object *mo = obj;
 
-	if( mo->name ) {
+	VIPS_FREE( mo->name );
-		vips_free( mo->name );
+	VIPS_FREEF( im_free_dmask, mo->mask );
-		mo->name = NULL;
-	}
-	if( mo->mask ) {
-		im_free_dmask( (DOUBLEMASK *) mo->mask );
-		mo->mask = NULL;
-	}
 
 	return( 0 );
 }
@@ -307,14 +301,8 @@ imask_dest( im_object obj )
 {
 	im_mask_object *mo = obj;
 
-	if( mo->name ) {
+	VIPS_FREE( mo->name );
-		vips_free( mo->name );
+	VIPS_FREEF( im_free_imask, mo->mask );
-		mo->name = NULL;
-	}
-	if( mo->mask ) {
-		im_free_imask( (INTMASK *) mo->mask );
-		mo->mask = NULL;
-	}
 
 	return( 0 );
 }
@@ -424,7 +412,7 @@ doublevec_dest( im_object obj )
 	im_doublevec_object *dv = obj;
 
 	if( dv->vec ) {
-		vips_free( dv->vec );
+		g_free( dv->vec );
 		dv->vec = NULL;
 		dv->n = 0;
 	}
@@ -509,7 +497,7 @@ intvec_dest( im_object obj )
 	im_intvec_object *iv = obj;
 
 	if( iv->vec ) {
-		vips_free( iv->vec );
+		g_free( iv->vec );
 		iv->vec = NULL;
 		iv->n = 0;
 	}
@@ -622,7 +610,7 @@ im_type_desc im__input_int = {
 static int
 input_string_init( im_object *obj, char *str )
 {
-	if( !(*obj = (im_object) vips_strdup( NULL, str )) ) 
+	if( !(*obj = (im_object) im_strdup( NULL, str )) ) 
 		return( -1 );
 
 	return( 0 );

libvips/deprecated/package.c

@@ -101,7 +101,7 @@ guess_prefix_vec( im_object *argv )
 		return( -1 );
 	}
 
-	argv[2] = vips_strdup( NULL, prefix );
+	argv[2] = im_strdup( NULL, prefix );
 
 	return( 0 );
 }
@@ -137,7 +137,7 @@ guess_libdir_vec( im_object *argv )
 		return( -1 );
 	}
 
-	argv[2] = vips_strdup( NULL, libdir );
+	argv[2] = im_strdup( NULL, libdir );
 
 	return( 0 );
 }
@@ -293,7 +293,7 @@ history_get_vec( im_object *argv )
 	const char *str;
 
 	if( !(str = im_history_get( (IMAGE *) argv[0] )) ||
-		!(*out = vips_strdup( NULL, str )) )
+		!(*out = im_strdup( NULL, str )) )
 		return( -1 );
 
 	return( 0 );
@@ -382,7 +382,7 @@ static im_arg_desc version_string_args[] = {
 static int
 version_string_vec( im_object *argv )
 {
-	if( !(argv[0] = vips_strdup( NULL, vips_version_string() )) )
+	if( !(argv[0] = im_strdup( NULL, vips_version_string() )) )
 		return( -1 );
 
 	return( 0 );
@@ -625,7 +625,7 @@ plugin_free( Plugin *plug )
 	}
 	VIPS_FREE( plug->name );
 	plug->pack = NULL;
-	vips_free( plug );
+	g_free( plug );
 
 	plugin_list = g_slist_remove( plugin_list, plug );
 
@@ -647,24 +647,17 @@ im_load_plugin( const char *name )
 
 	/* Build a new plugin.
 	 */
-	if( !(plug = VIPS_NEW( NULL, Plugin )) ) 
+	plug = VIPS_NEW( NULL, Plugin );
-		return( NULL );
 	plug->module = NULL;
-	plug->name = NULL;
+	plug->name = g_strdup( name );
 	plug->pack = NULL;
 	plugin_list = g_slist_prepend( plugin_list, plug );
 
-	/* Attach name.
-	 */
-	if( !(plug->name = vips_strdup( NULL, name )) ) {
-		plugin_free( plug );
-		return( NULL );
-	}
-
 	/* Open library.
 	 */
 	if( !(plug->module = g_module_open( name, 0 )) ) {
-		vips_error( "plugin", _( "unable to open plugin \"%s\"" ), name );
+		vips_error( "plugin", 
+			_( "unable to open plugin \"%s\"" ), name );
 		vips_error( "plugin", "%s", g_module_error() );
 		plugin_free( plug );
 

libvips/deprecated/vips7compat.c

@@ -120,8 +120,7 @@ im_add_callback( VipsImage *im,
 {
 	Callback *callback;
 
-	if( !(callback = VIPS_NEW( im, Callback )) )
+	callback = VIPS_NEW( VIPS_OBJECT( im ), Callback );
-		return( -1 );
 	callback->fn = fn;
 	callback->a = a;
 	callback->b = b;
@@ -144,8 +143,7 @@ im_add_callback1( VipsImage *im,
 {
 	Callback *callback;
 
-	if( !(callback = VIPS_NEW( im, Callback )) )
+	callback = VIPS_NEW( VIPS_OBJECT( im ), Callback );
-		return( -1 );
 	callback->fn = fn;
 	callback->a = a;
 	callback->b = b;
@@ -211,7 +209,7 @@ im_init( const char *filename )
 	VipsImage *image;
 
 	image = vips_image_new();
-	VIPS_SETSTR( image->filename, filename );
+	IM_SETSTR( image->filename, filename );
 
 	return( image );
 }
@@ -415,18 +413,9 @@ dupims( IMAGE *out, IMAGE **in )
 	IMAGE **new;
 	int i, n;
 
-	/* Count input images.
-	 */
 	for( n = 0; in[n]; n++ )
 		;
-
+	new = VIPS_ARRAY( VIPS_OBJECT( out ), n + 1, IMAGE * );
-	/* Allocate new array.
-	 */
-	if( !(new = VIPS_ARRAY( out, n + 1, IMAGE * )) )
-		return( NULL );
-	
-	/* Copy.
-	 */
 	for( i = 0; i < n; i++ )
 		new[i] = in[i];
 	new[n] = NULL;
@@ -466,7 +455,7 @@ dupims( IMAGE *out, IMAGE **in )
 int
 im_wrapmany( IMAGE **in, IMAGE *out, im_wrapmany_fn fn, void *a, void *b )
 {
-	Bundle *bun = VIPS_NEW( out, Bundle );
+	Bundle *bun;
 	int i, n;
 
 	/* Count input images.
@@ -480,7 +469,8 @@ im_wrapmany( IMAGE **in, IMAGE *out, im_wrapmany_fn fn, void *a, void *b )
 
 	/* Save args.
 	 */
-	if( !bun || !(in = dupims( out, in )) )
+	bun = VIPS_NEW( VIPS_OBJECT( out ), Bundle );
+	if( !(in = dupims( out, in )) )
 		return( -1 );
 	bun->fn = fn;
 	bun->a = a;
@@ -558,11 +548,12 @@ wrapone_gen( void **ins, void *out, int width, Bundle *bun, void *dummy )
 int
 im_wrapone( IMAGE *in, IMAGE *out, im_wrapone_fn fn, void *a, void *b )
 {
-	Bundle *bun = VIPS_NEW( out, Bundle );
+	Bundle *bun;
 	IMAGE *invec[2];
 
 	/* Heh, yuk. We cast back above.
 	 */
+	bun = VIPS_NEW( VIPS_OBJECT( out ), Bundle );
 	bun->fn = (im_wrapmany_fn) fn;
 	bun->a = a;
 	bun->b = b;
@@ -614,9 +605,10 @@ int
 im_wraptwo( IMAGE *in1, IMAGE *in2, IMAGE *out, 
 	im_wraptwo_fn fn, void *a, void *b )
 {
-	Bundle *bun = VIPS_NEW( out, Bundle );
+	Bundle *bun;
 	IMAGE *invec[3];
 
+	bun = VIPS_NEW( VIPS_OBJECT( out ), Bundle );
 	bun->fn = (im_wrapmany_fn) fn;
 	bun->a = a;
 	bun->b = b;

libvips/deprecated/wrapvips7.c

@@ -385,7 +385,7 @@ vips_wrap7_object_set_property( GObject *gobject,
 		break;
 
 	case VIPS_WRAP7_STRING:
-		VIPS_SETSTR( wrap7->vargv[i], g_value_get_string( value ) );
+		IM_SETSTR( wrap7->vargv[i], g_value_get_string( value ) );
 		break;
 
 	case VIPS_WRAP7_GVALUE:
@@ -852,3 +852,54 @@ vips__init_wrap7_classes( void )
 {
 	(void) im_map_packages( (VSListMap2Fn) vips_wrap7_build_package, NULL );
 }
+
+/* The vips7 malloc/free need to be thin wrappers over g_malloc()/g_free(),
+ * since we pass strings to and from the library and need to be able to free
+ * with either.
+ */
+
+int
+im_free( void *s )
+{        
+	g_free( s );
+
+	return( 0 );
+}
+
+void *
+im_malloc( IMAGE *im, size_t size )
+{
+	void *buf;
+
+	if( !(buf = g_try_malloc( size )) ) {
+		im_error( "im_malloc",
+			_( "out of memory --- size == %dMB" ),
+			(int) (size / (1024.0*1024.0))  );
+		im_warn( "im_malloc",
+			_( "out of memory --- size == %dMB" ),
+			(int) (size / (1024.0*1024.0))  );
+		return( NULL );
+	}
+
+	if( im && im_add_close_callback( im,
+		(im_callback_fn) im_free, buf, NULL ) ) {
+		im_free( buf );
+		return( NULL );
+	}
+
+	return( buf );
+}
+
+char *
+im_strdup( IMAGE *im, const char *str )
+{
+	int l = strlen( str );
+	char *buf;
+
+	if( !(buf = (char *) im_malloc( im, l + 1 )) )
+		return( NULL );
+	strcpy( buf, str );
+
+	return( buf );
+}
+

libvips/include/vips/header.h

@@ -97,7 +97,7 @@ int vips_image_copy_fieldsv( VipsImage *out, VipsImage *in1, ... )
 	__attribute__((sentinel));
 int vips_image_copy_fields( VipsImage *out, VipsImage *in );
 
-int vips_image_set( VipsImage *image, const char *field, GValue *value );
+void vips_image_set( VipsImage *image, const char *field, GValue *value );
 int vips_image_get( VipsImage *image, const char *field, GValue *value_copy );
 int vips_image_get_as_string( VipsImage *image, const char *field, char **out );
 GType vips_image_get_typeof( VipsImage *image, const char *field );
@@ -153,23 +153,23 @@ void *vips_blob_get( const GValue *value, size_t *length );
 int vips_blob_set( GValue *value, VipsCallbackFn free_fn, 
 	void *data, size_t length ); 
 
-int vips_image_set_area( VipsImage *image, 
+void vips_image_set_area( VipsImage *image, 
 	const char *field, VipsCallbackFn free_fn, void *data );
 int vips_image_get_area( VipsImage *image, const char *field, void **data );
-int vips_image_set_string( VipsImage *image, 
+void vips_image_set_string( VipsImage *image, 
 	const char *field, const char *str );
 int vips_image_get_string( VipsImage *image, const char *field, char **str );
-int vips_image_set_blob( VipsImage *image, const char *field, 
+void vips_image_set_blob( VipsImage *image, const char *field, 
 	VipsCallbackFn free_fn, void *data, size_t length );
 int vips_image_get_blob( VipsImage *image, const char *field, 
 	void **data, size_t *length );
 
 int vips_image_get_int( VipsImage *image, const char *field, int *out );
-int vips_image_set_int( VipsImage *image, const char *field, int i );
+void vips_image_set_int( VipsImage *image, const char *field, int i );
 int vips_image_get_double( VipsImage *image, const char *field, double *out );
-int vips_image_set_double( VipsImage *image, const char *field, double d );
+void vips_image_set_double( VipsImage *image, const char *field, double d );
 int vips_image_get_string( VipsImage *image, const char *field, char **out );
-int vips_image_set_string( VipsImage *image, 
+void vips_image_set_string( VipsImage *image, 
 	const char *field, const char *str );
 
 int vips_image_history_printf( VipsImage *image, const char *format, ... )

libvips/include/vips/memory.h

@@ -44,16 +44,7 @@ G_STMT_START { \
         } \
 } G_STMT_END
 
-/* Can't just use VIPS_FREEF(), we want the extra cast to void on the argument
+#define VIPS_FREE( S ) VIPS_FREEF( g_free, (S) );
- * to vips_free() to make sure we can work for "const char *" variables.
- */
-#define VIPS_FREE( S ) \
-G_STMT_START { \
-        if( S ) { \
-                (void) vips_free( (void *) (S) ); \
-                (S) = 0; \
-        } \
-} G_STMT_END
 
 #define VIPS_SETSTR( S, V ) \
 G_STMT_START { \
@@ -63,22 +54,25 @@ G_STMT_START { \
                 if( !(S) || !sst || strcmp( (S), sst ) != 0 ) { \
                         VIPS_FREE( S ); \
                         if( sst ) \
-                                (S) = vips_strdup( NULL, sst ); \
+                                (S) = g_strdup( sst ); \
                 } \
         } \
 } G_STMT_END
 
-#define VIPS_NEW( IM, T ) ((T *) vips_malloc( (IM), sizeof( T )))
+#define VIPS_NEW( OBJ, T ) \
-#define VIPS_ARRAY( IM, N, T ) ((T *) vips_malloc( (IM), (N) * sizeof( T )))
+	((T *) vips_malloc( VIPS_OBJECT( OBJ ), sizeof( T )))
+#define VIPS_ARRAY( OBJ, N, T ) \
+	((T *) vips_malloc( VIPS_OBJECT( OBJ ), (N) * sizeof( T )))
 
-void *vips_malloc( VipsImage *image, size_t size );
+void *vips_malloc( VipsObject *object, size_t size );
-int vips_free( void *s );
+char *vips_strdup( VipsObject *object, char *str );
+int vips_free( void *buf );
 
-char *vips_strdup( VipsImage *image, const char *str );
+void vips_tracked_free( void *s );
-
+void *vips_tracked_malloc( VipsObject *object, size_t size );
-size_t vips_alloc_get_mem( void );
+size_t vips_tracked_get_mem( void );
-size_t vips_alloc_get_mem_highwater( void );
+size_t vips_tracked_get_mem_highwater( void );
-unsigned int vips_alloc_get_allocs( void );
+int vips_tracked_get_allocs( void );
 
 #ifdef __cplusplus
 }

libvips/include/vips/vips7compat.h

@@ -93,12 +93,7 @@ extern "C" {
 #define IM_DEG VIPS_DEG
 #define IM_PI VIPS_PI
 #define IM_RINT VIPS_RINT
-#define IM_NEW VIPS_NEW
-#define IM_ARRAY VIPS_ARRAY
-#define IM_SETSTR VIPS_SETSTR
 #define IM_ABS VIPS_ABS
-#define IM_FREE VIPS_FREE
-#define IM_FREEF VIPS_FREEF
 #define IM_NUMBER VIPS_NUMBER
 #define IM_CLIP VIPS_CLIP
 #define IM_CLIP_UCHAR VIPS_CLIP_UCHAR
@@ -290,9 +285,46 @@ int im_generate( VipsImage *im,
 #define im__print_renders vips__print_renders
 #define im_cache vips_image_cache
 
-#define im_malloc vips_malloc
+/* vips_alloc() and friends are not the same, we need to keep these as C.
-#define im_free vips_free
+ */
-#define im_strdup vips_strdup
+char *im_strdup( IMAGE *im, const char *str );
+int im_free( void *s );
+void *im_malloc( IMAGE *im, size_t size );
+
+#define IM_FREEF( F, S ) \
+G_STMT_START { \
+        if( S ) { \
+                (void) F( (S) ); \
+                (S) = 0; \
+        } \
+} G_STMT_END
+
+/* Can't just use VIPS_FREEF(), we want the extra cast to void on the argument
+ * to vips_free() to make sure we can work for "const char *" variables.
+ */
+#define IM_FREE( S ) \
+G_STMT_START { \
+        if( S ) { \
+                (void) im_free( (void *) (S) ); \
+                (S) = 0; \
+        } \
+} G_STMT_END
+
+#define IM_SETSTR( S, V ) \
+G_STMT_START { \
+        const char *sst = (V); \
+	\
+        if( (S) != sst ) { \
+                if( !(S) || !sst || strcmp( (S), sst ) != 0 ) { \
+                        IM_FREE( S ); \
+                        if( sst ) \
+                                (S) = im_strdup( NULL, sst ); \
+                } \
+        } \
+} G_STMT_END
+
+#define IM_NEW( IM, T ) ((T *) im_malloc( (IM), sizeof( T )))
+#define IM_ARRAY( IM, N, T ) ((T *) im_malloc( (IM), (N) * sizeof( T )))
 
 #define im_incheck vips_image_wio_input
 #define im_outcheck vips_image_wio_output
@@ -390,20 +422,21 @@ int im_wrapmany( VipsImage **in, VipsImage *out,
 #define im_blob_get vips_blob_get
 #define im_blob_set vips_blob_set
 
-#define im_meta_set vips_image_set
+#define im_meta_set( A, B, C ) (vips_image_set( A, B, C ), 0)
 #define im_meta_remove vips_image_remove
 #define im_meta_get vips_image_get
 #define im_meta_get_typeof vips_image_get_typeof
 
-#define im_meta_set_int vips_image_set_int
+#define im_meta_set_int( A, B, C ) (vips_image_set_int( A, B, C ), 0)
 #define im_meta_get_int vips_image_get_int
-#define im_meta_set_double vips_image_set_double
+#define im_meta_set_double( A, B, C ) (vips_image_set_double( A, B, C ), 0)
 #define im_meta_get_double vips_image_get_double
-#define im_meta_set_area vips_image_set_area
+#define im_meta_set_area( A, B, C, D ) (vips_image_set_area( A, B, C, D ), 0)
 #define im_meta_get_area vips_image_get_area
-#define im_meta_set_string vips_image_set_string
+#define im_meta_set_string( A, B, C ) (vips_image_set_string( A, B, C ), 0)
 #define im_meta_get_string vips_image_get_string
-#define im_meta_set_blob vips_image_set_blob
+#define im_meta_set_blob( A, B, C, D, E ) \
+	(vips_image_set_blob( A, B, C, D, E ), 0)
 #define im_meta_get_blob vips_image_get_blob
 
 #define im_semaphore_t VipsSemaphore

libvips/iofuncs/cache.c

@@ -36,11 +36,14 @@
 
 	listen for invalidate
 
-	can we estimate the resource needs of operations and drop very 
+	drop on cache full
-	expensive ones first?
 
-		get vips_malloc()/_free() to track current usage, check that 
+	have a drop-all call for debugging leaks
-		as well as hash table size when looking for cache overflow
+
+	will we need to drop all on exit? unclear
+
+	what about delayed writes ... do we ever write in close? we shouldn't,
+	should do in evalend or written or somesuch
 
  */
 

libvips/iofuncs/header.c

@@ -283,8 +283,8 @@ meta_free( VipsMeta *meta )
 			g_slist_remove( meta->im->meta_traverse, meta );
 
 	g_value_unset( &meta->value );
-	VIPS_FREE( meta->field );
+	g_free( meta->field );
-	vips_free( meta );
+	g_free( meta );
 }
 
 static VipsMeta *
@@ -292,16 +292,11 @@ meta_new( VipsImage *image, const char *field, GValue *value )
 {
 	VipsMeta *meta;
 
-	if( !(meta = VIPS_NEW( NULL, VipsMeta )) )
+	meta = g_new( VipsMeta, 1 );
-		return( NULL );
 	meta->im = image;
 	meta->field = NULL;
 	memset( &meta->value, 0, sizeof( GValue ) );
-
+	meta->field = g_strdup( field );
-	if( !(meta->field = vips_strdup( NULL, field )) ) {
-		meta_free( meta );
-		return( NULL );
-	}
 
 	g_value_init( &meta->value, G_VALUE_TYPE( value ) );
 	g_value_copy( value, &meta->value );
@@ -492,8 +487,6 @@ meta_cp_field( VipsMeta *meta, VipsImage *dst )
 }
 #endif /*DEBUG*/
 
-	/* No way to return error here, sadly.
-	 */
 	meta_copy = meta_new( dst, meta->field, &meta->value );
 
 #ifdef DEBUG
@@ -659,21 +652,15 @@ vips_image_copy_fields( VipsImage *out, VipsImage *in )
  *
  * g_value_init( &value, G_TYPE_INT );
  * g_value_set_int( &value, 42 );
- *
+ * vips_image_set( image, field, &value );
- * if( vips_image_set( image, field, &value ) ) {
- *   g_value_unset( &value );
- *   return( -1 );
- * }
  * g_value_unset( &value );
  *
  * return( 0 );
  * ]|
  *
  * See also: vips_image_get().
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set( VipsImage *image, const char *field, GValue *value )
 {
 	VipsMeta *meta;
@@ -682,14 +669,11 @@ vips_image_set( VipsImage *image, const char *field, GValue *value )
 	g_assert( value );
 
 	meta_init( image );
-	if( !(meta = meta_new( image, field, value )) )
+	meta = meta_new( image, field, value );
-		return( -1 );
 
 #ifdef DEBUG
 	meta_sanity( image );
 #endif /*DEBUG*/
-
-	return( 0 );
 }
 
 /**
@@ -1197,20 +1181,6 @@ value_get_area_length( const GValue *value )
 	return( area->length );
 }
 
-/* Helpers for set/get. Write a value and destroy it.
- */
-static int
-meta_set_value( VipsImage *image, const char *field, GValue *value )
-{
-	if( vips_image_set( image, field, value ) ) {
-		g_value_unset( value );
-		return( -1 );
-	}
-	g_value_unset( value );
-
-	return( 0 );
-}
-
 static int
 meta_get_value( VipsImage *image, 
 	const char *field, GType type, GValue *value_copy )
@@ -1241,18 +1211,16 @@ meta_get_value( VipsImage *image,
  * no longer needs the metadata, it will be freed with @free_fn.
  *
  * See also: vips_image_get_double(), vips_image_set()
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set_area( VipsImage *image, const char *field, 
 	VipsCallbackFn free_fn, void *data )
 {
 	GValue value = { 0 };
 
 	value_set_area( free_fn, data, &value );
-
+	vips_image_set( image, field, &value );
-	return( meta_set_value( image, field, &value ) );
+	g_value_unset( &value );
 }
 
 /** 
@@ -1333,10 +1301,9 @@ vips_ref_string_set( GValue *value, const char *str )
 
 	g_assert( G_VALUE_TYPE( value ) == VIPS_TYPE_REF_STRING );
 
-	if( !(str_copy = vips_strdup( NULL, str )) )
+	str_copy = g_strdup( str );
-		return( -1 );
 	if( !(area = area_new( (VipsCallbackFn) vips_free, str_copy )) ) {
-		vips_free( str_copy );
+		g_free( str_copy );
 		return( -1 );
 	}
 
@@ -1541,10 +1508,8 @@ vips_blob_set( GValue *value,
  * function over vips_image_set() using an vips_blob.
  *
  * See also: vips_image_get_blob(), vips_image_set().
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set_blob( VipsImage *image, const char *field, 
 	VipsCallbackFn free_fn, void *data, size_t length )
 {
@@ -1552,8 +1517,8 @@ vips_image_set_blob( VipsImage *image, const char *field,
 
 	g_value_init( &value, VIPS_TYPE_BLOB );
 	vips_blob_set( &value, free_fn, data, length );
-
+	vips_image_set( image, field, &value );
-	return( meta_set_value( image, field, &value ) );
+	g_value_unset( &value );
 }
 
 /** 
@@ -1641,18 +1606,16 @@ vips_image_get_int( VipsImage *image, const char *field, int *out )
  * function over vips_image_set().
  *
  * See also: vips_image_get_int(), vips_image_set()
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set_int( VipsImage *image, const char *field, int i )
 {
 	GValue value = { 0 };
 
 	g_value_init( &value, G_TYPE_INT );
 	g_value_set_int( &value, i );
-
+	vips_image_set( image, field, &value );
-	return( meta_set_value( image, field, &value ) );
+	g_value_unset( &value );
 }
 
 /** 
@@ -1710,18 +1673,16 @@ vips_image_get_double( VipsImage *image, const char *field, double *out )
  * function over vips_image_set().
  *
  * See also: vips_image_get_double(), vips_image_set()
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set_double( VipsImage *image, const char *field, double d )
 {
 	GValue value = { 0 };
 
 	g_value_init( &value, G_TYPE_DOUBLE );
 	g_value_set_double( &value, d );
-
+	vips_image_set( image, field, &value );
-	return( meta_set_value( image, field, &value ) );
+	g_value_unset( &value );
 }
 
 /** 
@@ -1776,18 +1737,16 @@ vips_image_get_string( VipsImage *image, const char *field, char **out )
  * function over vips_image_set() using an vips_ref_string.
  *
  * See also: vips_image_get_double(), vips_image_set(), vips_ref_string
- *
- * Returns: 0 on success, -1 otherwise.
  */
-int
+void
 vips_image_set_string( VipsImage *image, const char *field, const char *str )
 {
 	GValue value = { 0 };
 
 	g_value_init( &value, VIPS_TYPE_REF_STRING );
 	vips_ref_string_set( &value, str );
-
+	vips_image_set( image, field, &value );
-	return( meta_set_value( image, field, &value ) );
+	g_value_unset( &value );
 }
 
 /** 

libvips/iofuncs/image.c

@@ -489,7 +489,7 @@ lazy_free_cb( VipsImage *image, Lazy *lazy )
 {
 	VIPS_DEBUG_MSG( "lazy_free: %p \"%s\"\n", lazy, lazy->filename );
 
-	VIPS_FREE( lazy->filename );
+	g_free( lazy->filename );
 	VIPS_UNREF( lazy->real );
 }
 
@@ -499,19 +499,15 @@ lazy_new( VipsImage *image,
 {
 	Lazy *lazy;
 
-	if( !(lazy = VIPS_NEW( image, Lazy )) )
+	lazy = g_new( Lazy, 1 );
-		return( NULL );
 	VIPS_DEBUG_MSG( "lazy_new: %p \"%s\"\n", lazy, filename );
 	lazy->image = image;
 	lazy->format = format;
-	lazy->filename = NULL;
+	lazy->filename = g_strdup( filename );
 	lazy->disc = disc;
 	lazy->real = NULL;
 	g_signal_connect( image, "close", G_CALLBACK( lazy_free_cb ), lazy );
 
-	if( !(lazy->filename = vips_strdup( NULL, filename )) )
-		return( NULL );
-
 	return( lazy );
 }
 
@@ -671,8 +667,7 @@ vips_image_open_lazy( VipsImage *image,
 {
 	Lazy *lazy;
 
-	if( !(lazy = lazy_new( image, format, filename, disc )) )
+	lazy = lazy_new( image, format, filename, disc );
-		return( -1 );
 
 	/* Read header fields to init the return image. THINSTRIP since this is
 	 * probably a disc file. We can't tell yet whether we will be opening
@@ -712,6 +707,9 @@ vips_image_save_cb( VipsImage *image, int *result, SaveBlock *sb )
 {
 	if( sb->save_fn( image, sb->filename ) )
 		*result = -1;
+
+	g_free( sb->filename );
+	g_free( sb );
 }
 
 static void
@@ -719,12 +717,11 @@ vips_attach_save( VipsImage *image, int (*save_fn)(), const char *filename )
 {
 	SaveBlock *sb;
 
-	if( (sb = VIPS_NEW( image, SaveBlock )) ) {
+	sb = g_new( SaveBlock, 1 );
-		sb->save_fn = save_fn;
+	sb->save_fn = save_fn;
-		sb->filename = vips_strdup( image, filename );
+	sb->filename = g_strdup( filename );
-		g_signal_connect( image, "written", 
+	g_signal_connect( image, "written", 
-			G_CALLBACK( vips_image_save_cb ), sb );
+		G_CALLBACK( vips_image_save_cb ), sb );
-	}
 }
 
 /* Progress feedback. 

libvips/iofuncs/memory.c

@@ -1,4 +1,4 @@
-/* : mem handling stuff
+/* tracked memory
  *
  * 2/11/99 JC
  *	- from im_open.c and callback.c
@@ -19,6 +19,9 @@
  *	- im_malloc()/im_free() now call g_try_malloc()/g_free() ... removes 
  *	  confusion over whether to use im_free() or g_free() for things like 
  *	  im_header_string()
+ * 21/9/11
+ * 	- rename as vips_tracked_malloc() to emphasise difference from
+ * 	  g_malloc()/g_free()
  */
 
 /*
@@ -70,20 +73,18 @@
  * @stability: Stable
  * @include: vips/vips.h
  *
- * Simple memory allocation utilities. These functions and macros help
+ * These functions cover two main areas.
- * allocate and free memory. Most of VIPS uses them, though some parts use
- * the g_malloc() system instead, confusingly.
  *
- * Use these functions for large allocations, such as arrays of image data. 
+ * First, some simple utility functions over the underlying
- * vips uses vips_alloc_get_mem(), which gives the amount of memory currently
+ * g_malloc()/g_free() functions. Memory allocated and freeded using these
- * allocated via these functions, to decide when to start dropping cache.
+ * functions is interchangeable with any other glib library.
  *
- * If you compile with %DEBUGM it will track allocations for you, though
+ * Second, a pair of functions, vips_tracked_malloc() and vips_tracked_free()
- * valgrind or dmalloc are better solutions.
+ * which are NOT compatible. If you g_free() memory that has been allocated
- */
+ * with vips_tracked_malloc() you will see crashes. The tracked functions are
-
+ * only suitable for large allocations internal to the library, for example
-/* Define for simple malloc tracking ... better to use dmalloc if you can.
+ * pixel buffers. libvips tracks the total amount of live tracked memory and
-#define DEBUGM
+ * uses this information to decide when to trim caches.
  */
 
 /* g_assert( 0 ) on memory errors.
@@ -94,22 +95,14 @@
 #  warning DEBUG on in libsrc/iofuncs/memory.c
 #endif /*DEBUG*/
 
-static size_t vips_alloc_mem = 0;
+static int vips_tracked_allocs = 0;
-static unsigned int vips_allocs = 0;
+static size_t vips_tracked_mem = 0;
-static size_t vips_alloc_mem_highwater = 0;
+static size_t vips_tracked_mem_highwater = 0;
-static GMutex *vips_alloc_mutex = NULL;
+static GMutex *vips_tracked_mutex = NULL;
-
-#ifdef DEBUGM
-/* Track total alloc/total free here for debugging.
- */
-static GSList *malloc_list = NULL;
-static const int trace_freq = 100;	/* Msg every this many malloc/free */
-static int next_trace = 0;
-#endif /*DEBUGM*/
 
 /**
  * VIPS_NEW:
- * @IM: allocate memory local to @IM, or %NULL for no auto-free
+ * @OBJ: allocate memory local to @OBJ, or %NULL for no auto-free
  * @T: type of thing to allocate
  *
  * Returns: A pointer of type @T *, or %NULL on error.
@@ -117,28 +110,108 @@ static int next_trace = 0;
 
 /**
  * VIPS_ARRAY:
- * @IM: allocate memory local to @IM, or %NULL for no auto-free
+ * @OBJ: allocate memory local to @OBJ, or %NULL for no auto-free
  * @N: number of @T 's to allocate
  * @T: type of thing to allocate
  *
  * Returns: A pointer of type @T *, or %NULL on error.
  */
 
+static void
+vips_malloc_cb( VipsObject *object, char *buf )
+{
+	g_free( buf );
+}
+
+/**
+ * vips_malloc:
+ * @object: allocate memory local to this #VipsObject, or %NULL
+ * @size: number of bytes to allocate
+ *
+ * g_malloc() local to @object, that is, the memory will be automatically 
+ * freed for you when the object is closed. If @object is %NULL, you need to 
+ * free the memory explicitly with g_free().
+ *
+ * This function cannot fail. See vips_tracked_malloc() if you are 
+ * allocating large amounts of memory.
+ *
+ * See also: vips_tracked_malloc().
+ *
+ * Returns: a pointer to the allocated memory
+ */
+void *
+vips_malloc( VipsObject *object, size_t size )
+{
+	void *buf;
+
+	buf = g_malloc( size );
+
+        if( object )
+		g_signal_connect( object, "postclose", 
+			G_CALLBACK( vips_malloc_cb ), buf );
+
+	return( buf );
+}
+
+/**
+ * vips_strdup:
+ * @object: allocate memory local to this #VipsObject, or %NULL
+ * @str: string to copy
+ *
+ * g_strdup() a string. When @object is freed, the string will be freed for
+ * you.  If @object is %NULL, you need to 
+ * free the memory explicitly with g_free().
+ *
+ * This function cannot fail. 
+ *
+ * See also: vips_malloc().
+ *
+ * Returns: a pointer to the allocated memory
+ */
+char *
+vips_strdup( VipsObject *object, char *str )
+{
+	char *str_dup;
+
+	str_dup = g_strdup( str );
+
+        if( object )
+		g_signal_connect( object, "postclose", 
+			G_CALLBACK( vips_malloc_cb ), str_dup );
+
+	return( str_dup );
+}
+
 /**
  * vips_free:
- * @s: memory to free
+ * @buf: memory to free
  *
- * VIPS free function. VIPS tries to use this instead of free(). It always
+ * Frees memory with g_free() and returns 0. Handy for callbacks.
- * returns zero, so it can be used as a callback handler. 
  *
- * Only use it to free
+ * See also: vips_malloc().
- * memory that was previously allocated with vips_malloc() with a %NULL first
- * argument.
  *
  * Returns: 0
  */
 int
-vips_free( void *s )
+vips_free( void *buf )
+{
+	g_free( buf );
+
+	return( 0 );
+}
+
+/**
+ * vips_tracked_free:
+ * @s: memory to free
+ *
+ * Only use it to free
+ * memory that was previously allocated with vips_tracked_malloc() with a 
+ * %NULL first argument.
+ *
+ * See also: vips_tracked_malloc().
+ */
+void
+vips_tracked_free( void *s )
 {
 	size_t size;
 
@@ -148,83 +221,63 @@ vips_free( void *s )
 	s = (void *) ((char*)s - 16);
 	size = *((size_t*)s);
 
-	g_mutex_lock( vips_alloc_mutex );
+	g_mutex_lock( vips_tracked_mutex );
 
-	if( vips_allocs <= 0 ) 
+	if( vips_tracked_allocs <= 0 ) 
-		vips_warn( "vips_malloc", 
+		vips_warn( "vips_tracked", 
 			"%s", _( "vips_free: too many frees" ) );
-	vips_alloc_mem -= size;
+	vips_tracked_mem -= size;
-	vips_allocs -= 1;
+	if( vips_tracked_mem < 0 ) 
+		vips_warn( "vips_tracked", 
+			"%s", _( "vips_free: too much free" ) );
+	vips_tracked_allocs -= 1;
 
-#ifdef DEBUGM
+	g_mutex_unlock( vips_tracked_mutex );
-	g_assert( g_slist_find( malloc_list, s ) );
-	malloc_list = g_slist_remove( malloc_list, s );
-	g_assert( !g_slist_find( malloc_list, s ) );
-	malloc_list = g_slist_remove( malloc_list, s );
-
-	next_trace += 1;
-	if( next_trace > trace_freq ) {
-		printf( "vips_free: %d, %d allocs, total %.3gM, "
-			"high water %.3gM\n", 
-			size,
-			vips_allocs,
-			vips_alloc_mem / (1024.0 * 1024.0), 
-			vips_alloc_mem_highwater / (1024.0 * 1024.0) );
-		next_trace = 0;
-	}
-#endif /*DEBUGM*/
-
-	g_mutex_unlock( vips_alloc_mutex );
-
-#ifdef DEBUG
-	if( !s )
-		g_assert( 0 );
-#endif /*DEBUG*/
 
 	g_free( s );
-
-	return( 0 );
-}
-
-static void
-vips_malloc_cb( VipsImage *image, char *buf )
-{
-	vips_free( buf );
 }
 
 /* g_mutex_new() is a macro.
  */
 static void *
-vips_alloc_mutex_new( void *data )
+vips_tracked_mutex_new( void *data )
 {
 	return( g_mutex_new() );
 }
 
+static void
+vips_tracked_cb( VipsObject *object, char *buf )
+{
+	vips_tracked_free( buf );
+}
+
 /**
- * vips_malloc:
+ * vips_tracked_malloc:
- * @image: allocate memory local to this #VipsImage, or %NULL
+ * @object: allocate memory local to this #VipsObject, or %NULL
  * @size: number of bytes to allocate
  *
- * Malloc local to @im, that is, the memory will be automatically 
+ * Malloc local to @object, that is, the memory will be automatically 
- * freed for you when the image is closed. If @im is %NULL, you need to free
+ * freed for you when the object is closed. If @object is %NULL, you need to 
- * the memory explicitly with vips_free().
+ * free the memory explicitly with vips_tracked_free().
- * If allocation fails vips_malloc() returns %NULL and 
+ *
+ * If allocation fails, vips_malloc() returns %NULL and 
  * sets an error message.
  *
- * If two threads try to allocate local to the same @im at the same time, you 
+ * You must only free the memory returned with vips_tracked_free().
- * can get heap corruption. 
+ *
+ * See also: vips_tracked_free(), vips_malloc().
  *
  * Returns: a pointer to the allocated memory, or %NULL on error.
  */
 void *
-vips_malloc( VipsImage *image, size_t size )
+vips_tracked_malloc( VipsObject *object, size_t size )
 {
-	static GOnce vips_alloc_once = G_ONCE_INIT;
+	static GOnce vips_tracked_once = G_ONCE_INIT;
 
         void *buf;
 
-	vips_alloc_mutex = g_once( &vips_alloc_once, 
+	vips_tracked_mutex = g_once( &vips_tracked_once, 
-		vips_alloc_mutex_new, NULL );
+		vips_tracked_mutex_new, NULL );
 
 	/* Need an extra sizeof(size_t) bytes to track 
 	 * size of this block. Ask for an extra 16 to make sure we don't break
@@ -237,75 +290,35 @@ vips_malloc( VipsImage *image, size_t size )
 		g_assert( 0 );
 #endif /*DEBUG*/
 
-		vips_error( "vips_malloc", 
+		vips_error( "vips_tracked", 
 			_( "out of memory --- size == %dMB" ), 
 			(int) (size / (1024.0*1024.0))  );
-		vips_warn( "vips_malloc", 
+		vips_warn( "vips_tracked", 
 			_( "out of memory --- size == %dMB" ), 
 			(int) (size / (1024.0*1024.0))  );
 
                 return( NULL );
 	}
 
-	g_mutex_lock( vips_alloc_mutex );
+	g_mutex_lock( vips_tracked_mutex );
-
-#ifdef DEBUGM
-	g_assert( !g_slist_find( malloc_list, buf ) );
-	malloc_list = g_slist_prepend( malloc_list, buf );
-#endif /*DEBUGM*/
 
 	*((size_t *)buf) = size;
 	buf = (void *) ((char *)buf + 16);
 
-	vips_alloc_mem += size;
+	vips_tracked_mem += size;
-	if( vips_alloc_mem > vips_alloc_mem_highwater ) 
+	if( vips_tracked_mem > vips_tracked_mem_highwater ) 
-		vips_alloc_mem_highwater = vips_alloc_mem;
+		vips_tracked_mem_highwater = vips_tracked_mem;
-	vips_allocs += 1;
+	vips_tracked_allocs += 1;
 
-#ifdef DEBUGM
+	g_mutex_unlock( vips_tracked_mutex );
-	next_trace += 1;
-	if( next_trace > trace_freq ) {
-		printf( "vips_malloc: %d, %d allocs, total %.3gM, "
-			"high water %.3gM\n", 
-			size, 
-			vips_allocs,
-			vips_alloc_mem / (1024.0 * 1024.0),
-			vips_alloc_mem_highwater / (1024.0 * 1024.0) );
-		next_trace = 0;
-	}
-#endif /*DEBUGM*/
 
-	g_mutex_unlock( vips_alloc_mutex );
+        if( object )
-
+		g_signal_connect( object, "postclose", 
-#ifdef DEBUGM
+			G_CALLBACK( vips_tracked_cb ), buf );
-	/* Handy to breakpoint on this printf() for catching large mallocs().
-	 */
-	if( size > 1000000 ) 
-		printf( "woah! big!\n" );
-#endif /*DEBUGM*/
- 
-        if( image )
-		g_signal_connect( image, "postclose", 
-			G_CALLBACK( vips_malloc_cb ), buf );
 
         return( buf );
 }
 
-/* strdup local to a descriptor.
- */
-char *
-vips_strdup( VipsImage *image, const char *str )
-{
-	int l = strlen( str );
-	char *buf;
-
-	if( !(buf = (char *) vips_malloc( image, l + 1 )) )
-		return( NULL );
-	strcpy( buf, str );
-
-	return( buf );
-}
-
 /**
  * vips_alloc_get_mem:
  *
@@ -316,13 +329,13 @@ vips_strdup( VipsImage *image, const char *str )
  * Returns: the number of currently allocated bytes
  */
 size_t
-vips_alloc_get_mem( void )
+vips_tracked_get_mem( void )
 {
-	return( vips_alloc_mem );
+	return( vips_tracked_mem );
 }
 
 /**
- * vips_alloc_get_mem_highwater:
+ * vips_tracked_get_mem_highwater:
  *
  * Returns the largest number of bytes simultaneously allocated via 
  * vips_malloc() and friends. 
@@ -330,21 +343,21 @@ vips_alloc_get_mem( void )
  * Returns: the largest number of currently allocated bytes
  */
 size_t
-vips_alloc_get_mem_highwater( void )
+vips_tracked_get_mem_highwater( void )
 {
-	return( vips_alloc_mem_highwater );
+	return( vips_tracked_mem_highwater );
 }
 
 /**
- * vips_alloc_get_allocs:
+ * vips_tracked_get_allocs:
  *
  * Returns the number active allocations. 
  *
  * Returns: the number active allocations
  */
-unsigned int
+int
-vips_alloc_get_allocs( void )
+vips_tracked_get_allocs( void )
 {
-	return( vips_allocs );
+	return( vips_tracked_allocs );
 }
 

libvips/iofuncs/vips.c

@@ -537,11 +537,7 @@ rebuild_header_meta( VipsImage *im, xmlNode *i )
 					"save format" ) );
 				return( -1 );
 			}
-			if( vips_image_set( im, name, &value ) ) {
+			vips_image_set( im, name, &value );
-				g_value_unset( &save_value );
-				g_value_unset( &value );
-				return( -1 );
-			}
 			g_value_unset( &save_value );
 			g_value_unset( &value );
 		}
@@ -615,11 +611,8 @@ readhist( VipsImage *im )
 
 		if( !(doc = read_xml( im )) )
 			return( -1 );
-		if( vips_image_set_area( im, VIPS_META_XML, 
+		vips_image_set_area( im, VIPS_META_XML, 
-			(VipsCallbackFn) xmlFreeDoc, doc ) ) {
+			(VipsCallbackFn) xmlFreeDoc, doc );
-			xmlFreeDoc( doc );
-			return( -1 );
-		}
 	}
 
 	if( rebuild_header( im ) )