From e99f6cc49eb226b16d5629a437f30896ab5e0971 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Tue, 6 Sep 2011 10:37:00 +0100
Subject: [PATCH] fix up VipsPool

vipspool working, test program in ~/try, passes valgrind
---
 ChangeLog                   |  1 +
 TODO                        |  3 ---
 libvips/include/vips/pool.h | 14 +++++++-----
 libvips/iofuncs/pool.c      | 43 +++++++++++++++++++++++++++----------
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 36fb2dd6..f8ef4db3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,7 @@
 - VipsMin stops search early if it can
 - C API supports optional output args
 - switch back to int-valued operations
+- fix up VipsPool
 
 10/8/11 started 7.26.3
 - don't use G_VALUE_COLLECT_INIT(), many platforms do not have a glib this
diff --git a/TODO b/TODO
index 4ea95bb2..a58c6257 100644
--- a/TODO
+++ b/TODO
@@ -1,6 +1,3 @@
-- revise vipspool, write a test prog
-
-
 
 
 - could we generate the code for vips_add() automatically? it might be nice to
diff --git a/libvips/include/vips/pool.h b/libvips/include/vips/pool.h
index b6fa4285..6c4587e8 100644
--- a/libvips/include/vips/pool.h
+++ b/libvips/include/vips/pool.h
@@ -60,6 +60,10 @@ typedef struct _VipsPool {
 	/* A table of all the contexts we've seen.
 	 */
 	GHashTable *contexts;
+
+	/* Track a name for debugging.
+	 */
+	const char *name;
 } VipsPool;
 
 typedef struct _VipsPoolClass {
@@ -72,12 +76,12 @@ VipsPool *vips_pool_new( const char *name );
 VipsPoolContext *vips_pool_context_new( VipsPool *pool );
 GObject **vips_pool_context_object( VipsPoolContext *context, int n );
 
-/* Save some typing.
+/* Save some typing. This assumes you have a (VipsPoolContext *) called
+ * "context" in scope.
  */
-#define VIPS_VAR_IMAGE_REF( N ) \
-	((VipsImage **) vips_pool_context_object( context, (N) ))
-#define VIPS_VAR_IMAGE( N ) \
-	(*((VipsImage **) vips_pool_context_object( context, (N) )))
+#define VIPS_VI( N ) \
+        (*((VipsImage **) vips_pool_context_object( context, (N) )))
+
 
 #ifdef __cplusplus
 }
diff --git a/libvips/iofuncs/pool.c b/libvips/iofuncs/pool.c
index 603c667c..069dacda 100644
--- a/libvips/iofuncs/pool.c
+++ b/libvips/iofuncs/pool.c
@@ -28,9 +28,7 @@
  */
 
 /*
-#define DEBUG
 #define VIPS_DEBUG
-#define DEBUG_REF
  */
 
 #ifdef HAVE_CONFIG_H
@@ -83,16 +81,16 @@
 	{
 		VipsPoolContext *context = vips_pool_context_new( pool );
 
-		if( vips_add( in1, in2, VIPS_VAR_IMAGE_REF( 1 ), NULL ) ||
-			vips_add( in1, VIPS_VAR_IMAGE( 1 ), out, NULL ) )
+		if( vips_add( in1, in2, &VIPS_VI( 1 ), NULL ) ||
+			vips_add( in1, VIPS_VI( 1 ), out, NULL ) )
 			return( -1 );
 
 		return( 0 );
 	}
 
   vips_pool_context_new() creates a new context to hold a set of temporary 
-  objects. You can get a reference to a temporary image object with the 
-  macro VIPS_VAR_IMAGE_REF(), and get the object with VIPS_VAR_IMAGE(). 
+  objects. You can get a pointer to a temporary image object with the 
+  macro VIPS_VI() (this assumes there is a variable called "context" in scope).
   Temporary objects are numbered from zero.
 
   Our caller will (eventually) call g_object_unref() on the pool and this 
@@ -118,10 +116,23 @@ vips_pool_dispose( GObject *gobject )
 }
 
 static void
-vips_pool_context_print( VipsPoolContext *context, VipsBuf *buf )
+vips_pool_context_print( VipsPoolContext *context, VipsPoolContext *value, 
+	VipsBuf *buf )
 {
-	vips_buf_appendf( buf, "VipsPoolContext %p, %d objects\n", 
-		context, context->len );
+	int i, n;
+
+	n = 0;
+	for( i = 0; i < context->len; i++ )
+		if( g_ptr_array_index( context, i ) ) 
+			n += 1;
+
+	vips_buf_appendf( buf, "VipsPoolContext %p, size %d, %d objects\n", 
+		context, context->len, n );
+
+	for( i = 0; i < context->len; i++ )
+		if( g_ptr_array_index( context, i ) ) 
+			vips_buf_appendf( buf, "\t%p\n", 
+				g_ptr_array_index( context, i ) ) ;
 }
 
 static void
@@ -166,7 +177,7 @@ vips_pool_new( const char *name )
 
 	pool = VIPS_POOL( g_object_new( VIPS_TYPE_POOL, NULL ) );
 
-	g_object_set( pool, "name", name, NULL );
+	pool->name = name;
 
 	if( vips_object_build( VIPS_OBJECT( pool ) ) ) {
 		VIPS_UNREF( pool );
@@ -176,12 +187,22 @@ vips_pool_new( const char *name )
 	return( pool );
 }
 
+static void
+vips_pool_context_element_free( GObject *object )
+{
+	if( object )
+		g_object_unref( object );
+}
+
 VipsPoolContext *
 vips_pool_context_new( VipsPool *pool )
 {
 	VipsPoolContext *context;
 
-	context = g_ptr_array_new_with_free_func( g_object_unref );
+	/* g_object_unref() hates unreffing NULL.
+	 */
+	context = g_ptr_array_new_with_free_func( 
+		(GDestroyNotify) vips_pool_context_element_free );
 	g_hash_table_insert( pool->contexts, context, context );
 
 	return( context );