more work on refs

TODO

@@ -1,3 +1,26 @@
+- call:
+
+	vips_object_unref_outputs( object );
+
+  in more places .. a good test is
+	
+	vips copy wtc.jpg x.v
+
+  where wtc.jpg is an empty file
+
+  we need to catch all the error paths out of VipsForeign
+
+  would catching in vips_call_split() be enough?
+
+
+
+
+- we also have vips_argument_dispose_all() and call it from vips_call()
+  vips_call_split(), does this do the same job?
+
+
+
+
 - test vips_foreign_load_vips_get_flags(), sense inverted?
 
   we have various printf()s left in the byteswap codepath

libvips/arithmetic/arithmetic.c

@@ -311,11 +311,12 @@ vips_arithmetic_build( VipsObject *object )
 	printf( "\n" );
 #endif /*DEBUG*/
 
-	g_object_set( arithmetic, "out", vips_image_new(), NULL ); 
-
-	if( VIPS_OBJECT_CLASS( vips_arithmetic_parent_class )->build( object ) )
+	if( VIPS_OBJECT_CLASS( vips_arithmetic_parent_class )->
+		build( object ) ) 
 		return( -1 );
 
+	g_object_set( arithmetic, "out", vips_image_new(), NULL ); 
+
 	/* No need to check input bands, bandalike will do this for us.
 	 */
 	if( arithmetic->n > MAX_INPUT_IMAGES ) {
@@ -326,9 +327,9 @@ vips_arithmetic_build( VipsObject *object )
 	for( i = 0; i < arithmetic->n; i++ )
 		if( vips_image_pio_input( arithmetic->in[i] ) || 
 			vips_check_uncoded( "VipsArithmetic", 
-				arithmetic->in[i] ) )
+				arithmetic->in[i] ) ) 
 			return( -1 );
-	if( vips_image_pio_output( arithmetic->out ) )
+	if( vips_image_pio_output( arithmetic->out ) ) 
 		return( -1 );
 
 	format = (VipsImage **) 
@@ -343,14 +344,14 @@ vips_arithmetic_build( VipsObject *object )
 	if( vips__formatalike_vec( arithmetic->in, format, arithmetic->n ) ||
 		vips__bandalike_vec( "VipsArithmetic", 
 			format, band, arithmetic->n, arithmetic->base_bands ) ||
-		vips__sizealike_vec( band, size, arithmetic->n ) )
+		vips__sizealike_vec( band, size, arithmetic->n ) ) 
 		return( -1 );
 
 	/* Keep a copy of the processed images here for subclasses.
 	 */
 	arithmetic->ready = size;
 
-	if( vips_image_copy_fields_array( arithmetic->out, size ) )
+	if( vips_image_copy_fields_array( arithmetic->out, size ) ) 
 		return( -1 );
         vips_demand_hint_array( arithmetic->out, 
 		VIPS_DEMAND_STYLE_THINSTRIP, size );
@@ -360,7 +361,7 @@ vips_arithmetic_build( VipsObject *object )
 
 	if( vips_image_generate( arithmetic->out,
 		vips_start_many, vips_arithmetic_gen, vips_stop_many, 
-		arithmetic->ready, arithmetic ) )
+		arithmetic->ready, arithmetic ) ) 
 		return( -1 );
 
 	return( 0 );

libvips/foreign/foreign.c

@@ -492,10 +492,8 @@ vips_foreign_load_start_cb( VipsImage *out, void *a, void *dummy )
 		/* Read the image in.
 		 */
 		if( class->load( load ) ||
-			vips_image_pio_input( load->real ) ) {
-			VIPS_UNREF( load->real );
+			vips_image_pio_input( load->real ) ) 
 			return( NULL );
-		}
 	}
 
 	return( vips_region_new( load->real ) );
@@ -530,8 +528,6 @@ vips_foreign_load_build( VipsObject *object )
 	VipsForeignLoad *load = VIPS_FOREIGN_LOAD( object );
 	VipsForeignLoadClass *class = VIPS_FOREIGN_LOAD_GET_CLASS( object );
 
-	g_object_set( object, "out", vips_image_new(), NULL ); 
-
 	if( class->get_flags &&
 		class->get_flags( load ) )
 		return( -1 );
@@ -540,10 +536,12 @@ vips_foreign_load_build( VipsObject *object )
 		build( object ) )
 		return( -1 );
 
+	g_object_set( object, "out", vips_image_new(), NULL ); 
+
 	/* Read the header into @out.
 	 */
 	if( class->header &&
-		class->header( load ) )
+		class->header( load ) ) 
 		return( -1 );
 
 	/* If there's no ->load() method then the header read has done
@@ -565,7 +563,7 @@ vips_foreign_load_build( VipsObject *object )
 			vips_foreign_load_start_cb, 
 			vips_foreign_load_generate_cb, 
 			vips_stop_one, 
-			load, NULL ) )
+			load, NULL ) ) 
 			return( -1 );
 	}
 

libvips/include/vips/object.h

@@ -483,6 +483,8 @@ void vips_object_sanity_all( void );
 
 void vips_object_rewind( VipsObject *object );
 
+void vips_object_unref_outputs( VipsObject *object );
+
 #ifdef __cplusplus
 }
 #endif /*__cplusplus*/

libvips/iofuncs/object.c

@@ -1998,3 +1998,42 @@ vips_object_sanity_all( void )
 	vips_object_map( 
 		(VipsSListMap2Fn) vips_object_sanity_all_cb, NULL, NULL );
 }
+
+static void *
+vips_object_unref_outputs_sub( VipsObject *object,
+	GParamSpec *pspec,
+	VipsArgumentClass *argument_class,
+	VipsArgumentInstance *argument_instance,
+	void *a, void *b )
+{
+	if( (argument_class->flags & VIPS_ARGUMENT_OUTPUT) &&
+		G_IS_PARAM_SPEC_OBJECT( pspec ) &&
+		argument_instance->assigned ) {
+		GObject *value;
+
+		g_object_get( object, 
+			g_param_spec_get_name( pspec ), &value, NULL );
+
+		/* Doing the get refs the object, so unref the get, then unref
+		 * again since this an an output object of the operation.
+		 */
+		g_object_unref( value );
+		g_object_unref( value );
+	}
+
+	return( NULL );
+}
+
+/* Unref all assigned output objects.
+ *
+ * After an object is built, all output args are owned by the caller. If
+ * something goes wrong before then, we have to unref the outputs that have
+ * been made so far. And this function can also be useful for callers when
+ * they've finished processing outputs themselves.
+ */
+void
+vips_object_unref_outputs( VipsObject *object )
+{
+	(void) vips_argument_map( object,
+		vips_object_unref_outputs_sub, NULL, NULL );
+}

libvips/iofuncs/operation.c

@@ -851,36 +851,12 @@ vips_call_argv_output( VipsObject *object,
 	return( NULL );
 }
 
-static void *
-vips_call_argv_unref_output( VipsObject *object,
-	GParamSpec *pspec,
-	VipsArgumentClass *argument_class,
-	VipsArgumentInstance *argument_instance,
-	void *a, void *b )
-{
-	if( (argument_class->flags & VIPS_ARGUMENT_OUTPUT) &&
-		G_IS_PARAM_SPEC_OBJECT( pspec ) &&
-		argument_instance->assigned ) {
-		GObject *value;
-
-		g_object_get( object, 
-			g_param_spec_get_name( pspec ), &value, NULL );
-
-		/* Doing the get refs the object, so unref the get, then unref
-		 * again since this an an output object of the operation.
-		 */
-		g_object_unref( value );
-		g_object_unref( value );
-	}
-
-	return( NULL );
-}
-
 /* Our main command-line entry point. Optional args should have been set by
  * the GOption parser already, see above.
  *
  * We don't create the operation, so we must not unref it. The caller must
- * unref on error too.
+ * unref on error too. The caller must also call vips_object_unref_outputs() on
+ * all code paths.
  */
 int
 vips_call_argv( VipsOperation *operation, int argc, char **argv )
@@ -907,15 +883,8 @@ vips_call_argv( VipsOperation *operation, int argc, char **argv )
 
 	call.i = 0;
 	if( vips_argument_map( VIPS_OBJECT( operation ),
-		vips_call_argv_input, &call, NULL ) ) {
-		/* We must unref any output objects, they are holding refs to
-		 * the operation.
-		 */
-		(void) vips_argument_map( VIPS_OBJECT( operation ),
-			vips_call_argv_unref_output, NULL, NULL );
-
+		vips_call_argv_input, &call, NULL ) ) 
 		return( -1 );
-	}
 
 	/* Any unused arguments? We must fail. Consider eg. "vips bandjoin a b
 	 * c". This would overwrite b with a and ignore c, potentially
@@ -924,37 +893,16 @@ vips_call_argv( VipsOperation *operation, int argc, char **argv )
 	if( argc > call.i ) {
 		vips_error( VIPS_OBJECT_GET_CLASS( operation )->nickname, 
 			"%s", _( "too many arguments" ) );
-
-		/* We must unref any output objects, they are holding refs to
-		 * the operation.
-		 */
-		(void) vips_argument_map( VIPS_OBJECT( operation ),
-			vips_call_argv_unref_output, NULL, NULL );
-
 		return( -1 );
 	}
 
-	if( vips_object_build( VIPS_OBJECT( operation ) ) ) {
-		/* We must unref any output objects, they are holding refs to
-		 * the operation.
-		 */
-		(void) vips_argument_map( VIPS_OBJECT( operation ),
-			vips_call_argv_unref_output, NULL, NULL );
-
+	if( vips_object_build( VIPS_OBJECT( operation ) ) ) 
 		return( -1 );
-	}
 
 	call.i = 0;
 	if( vips_argument_map( VIPS_OBJECT( operation ),
-		vips_call_argv_output, &call, NULL ) ) {
-		(void) vips_argument_map( VIPS_OBJECT( operation ),
-			vips_call_argv_unref_output, NULL, NULL );
-
+		vips_call_argv_output, &call, NULL ) ) 
 		return( -1 );
-	}
-
-	(void) vips_argument_map( VIPS_OBJECT( operation ),
-		vips_call_argv_unref_output, NULL, NULL );
 
 	return( 0 );
 }

tools/vips.c

@@ -1075,10 +1075,13 @@ main( int argc, char **argv )
 			if( argc == 1 ) 
 				vips_object_print( VIPS_OBJECT( operation ) );
 
+			vips_object_unref_outputs( VIPS_OBJECT( operation ) );
 			g_object_unref( operation );
+
 			error_exit( NULL );
 		}
 
+		vips_object_unref_outputs( VIPS_OBJECT( operation ) );
 		g_object_unref( operation );
 
 		handled = TRUE;