get im_system_image() working again

chop stuff about to get system image working again.

TODO

@@ -1,5 +1,4 @@
 
-
 - revisit orc conv
 
   use an 8.8 accumulator ... build the scale into the 8.8 coeffs ... no div at

libvips/conversion/im_system_image.c

@@ -75,7 +75,9 @@ system_image( IMAGE *im,
 		if( !vips_buf_appends( &buf, line ) )
 			break; 
 
-	result = pclose( fp );
+	if( (result = pclose( fp )) ) 
+		im_error( "im_system_image", 
+			_( "command failed: \"%s\"" ), cmd_format );
 
 	if( log )
 		*log = im_strdup( NULL, vips_buf_all( &buf ) );
@@ -152,6 +154,7 @@ im_system_image( IMAGE *im,
 	if( system_image( im, in_image, out_name, cmd_format, log ) ) {
 		im_close( in_image );
 		g_free( out_name );
+		g_unlink( out_name );
 
 		return( NULL );
 	}
@@ -159,18 +162,13 @@ im_system_image( IMAGE *im,
 
 	if( !(out = im_open( out_name, "r" )) ) {
 		g_free( out_name );
-
-		return( NULL );
-	}
-	if( im_add_postclose_callback( out, 
-		(im_callback_fn) unlink, out->filename, NULL ) ) {
-		g_free( out_name );
-		im_close( out );
 		g_unlink( out_name );
 
 		return( NULL );
 	}
 	g_free( out_name );
 
+	vips_image_set_delete_on_close( out, TRUE );
+
 	return( out );
 }

libvips/include/vips/image.h

@@ -197,7 +197,7 @@ typedef struct _VipsImage {
 	size_t length;		/* size of mmap area */
 	guint32 magic;		/* magic from header, endian-ness of image */
 
-	/* Partial image stuff. All private! All these fields are initialised 
+	/* Partial image stuff. All these fields are initialised 
 	 * to NULL and ignored unless set by vips_image_generate() etc.
 	 */
 	void *(*start)();	/* user-supplied start function */
@@ -264,6 +264,16 @@ typedef struct _VipsImage {
 	 */
 	gboolean hint_set;
 
+	/* Delete-on-close is hard to do with signals and callbacks since we
+	 * really need to do this in finalize after the fd has been closed,
+	 * but you can't emit signals then.
+	 *
+	 * Also keep a private copy of the filename string to be deleted,
+	 * since image->filename will be freed in _dispose().
+	 */
+	gboolean delete_on_close;
+	char *delete_on_close_filename;
+
 } VipsImage;
 
 typedef struct _VipsImageClass {
@@ -363,6 +373,8 @@ VipsImage *vips_image_new_from_file_raw( const char *filename,
 	int xsize, int ysize, int bands, int offset );
 VipsImage *vips_image_new_from_memory( void *buffer, 
 	int xsize, int ysize, int bands, VipsBandFormat bandfmt );
+void vips_image_set_delete_on_close( VipsImage *image, 
+	gboolean delete_on_close );
 VipsImage *vips_image_new_disc_temp( const char *format );
 int vips_image_write( VipsImage *image, const char *filename );
 

libvips/include/vips/object.h

@@ -160,7 +160,7 @@ typedef void *(*VipsArgumentMapFn)( VipsObject *, GParamSpec *,
 	VipsArgumentClass *, VipsArgumentInstance *, void *a, void *b );
 void *vips_argument_map( VipsObject *object, 
 	VipsArgumentMapFn fn, void *a, void *b );
-void vips_argument_free_all( VipsObject *object );
+void vips_argument_dispose_all( VipsObject *object );
 
 /* We have to loop over an objects args in several places, and we can't always
  * use vips_argument_map(), the preferred looper. Have the loop code as a

libvips/iofuncs/image.c

@@ -346,8 +346,7 @@ vips_image_finalize( GObject *gobject )
 			(void) vips__writehist( image );
 		if( close( image->fd ) == -1 ) 
 			vips_error( "VipsImage", 
-				_( "unable to close fd for %s" ), 
+				"%s", _( "unable to close fd" ) );
-				image->filename );
 		image->fd = -1;
 	}
 
@@ -366,8 +365,15 @@ vips_image_finalize( GObject *gobject )
 		image->data = NULL;
 	}
 
-	VIPS_FREE( image->filename );
+	if( image->delete_on_close ) {
-	VIPS_FREE( image->mode );
+		g_assert( image->delete_on_close_filename );
+
+		VIPS_DEBUG_MSG( "vips_image_finalize: removing temp %s\n", 
+				image->delete_on_close_filename );
+		g_unlink( image->delete_on_close_filename );
+		VIPS_FREE( image->delete_on_close_filename );
+		image->delete_on_close = FALSE;
+	}
 
 	VIPS_FREEF( g_mutex_free, image->sslock );
 
@@ -542,7 +548,7 @@ vips_image_rewind( VipsObject *object )
 	char *filename;
 	char *mode;
 
-	/* The old values for filename and mode become the new defaults.
+	/* This triggers a dispose. Copy filename/mode across the dispose.
 	 */
 	filename = g_strdup( vips_image_get_filename( image ) );
 	mode = g_strdup( vips_image_get_mode( image ) );
@@ -1454,7 +1460,7 @@ vips_image_set_kill( VipsImage *image, gboolean kill )
 	image->kill = kill;
 }
 
-/* Make a name for a filename-less image. Use immediately, don'#t free the
+/* Make a name for a filename-less image. Use immediately, don't free the
  * result.
  */
 static const char *
@@ -1747,12 +1753,29 @@ vips_image_new_from_memory( void *buffer,
 	return( image );
 }
 
-static void
+/**
-vips_image_new_temp_cb( VipsImage *image )
+ * vips_image_set_delete_on_close:
+ * @image: image to set
+ * @delete_on_close: format of file
+ *
+ * Sets the delete_on_close flag for the image. If this flag is set, when
+ * @image is finalized the filename held in @image->filename at the time of
+ * this call is unlinked.
+ *
+ * This function is clearly extremely dangerous, use with great caution.
+ *
+ * See also: vips__temp_name(), vips_image_new_disc_temp().
+ */
+void
+vips_image_set_delete_on_close( VipsImage *image, gboolean delete_on_close )
 {
-	g_assert( image->filename );
+	VIPS_DEBUG_MSG( "vips_image_set_delete_on_close: %d %s\n", 
+			delete_on_close, image->filename );
 
-	g_unlink( image->filename );
+	image->delete_on_close = delete_on_close;
+	VIPS_FREE( image->delete_on_close_filename );
+	if( delete_on_close ) 
+		VIPS_SETSTR( image->delete_on_close_filename, image->filename );
 }
 
 /**
@@ -1783,11 +1806,7 @@ vips_image_new_disc_temp( const char *format )
 	}
 	g_free( name );
 
-	/* Needs to be postclose so we can rewind after write without
+	vips_image_set_delete_on_close( image, TRUE );
-	 * deleting the file.
-	 */
-	g_signal_connect( image, "postclose", 
-		G_CALLBACK( vips_image_new_temp_cb ), NULL );
 
 	return( image );
 }

libvips/iofuncs/object.c

@@ -395,27 +395,21 @@ vips_object_dispose_argument( VipsObject *object, GParamSpec *pspec,
 	VipsArgumentInstance *argument_instance,
 	void *a, void *b )
 {
-#ifdef DEBUG
-	printf( "vips_object_dispose_argument: " );
-	vips_object_print_name( object );
-	printf( ".%s\n", g_param_spec_get_name( pspec ) ); 
-#endif /*DEBUG*/
-
 	g_assert( ((VipsArgument *) argument_class)->pspec == pspec );
 	g_assert( ((VipsArgument *) argument_instance)->pspec == pspec );
 
-	if( G_IS_PARAM_SPEC_STRING( pspec ) ) {
+	if( G_IS_PARAM_SPEC_OBJECT( pspec ) )
-		char **member = &G_STRUCT_MEMBER( char *, object,
-			argument_class->offset );
-
-		VIPS_FREE( *member );
-	}
-	else if( G_IS_PARAM_SPEC_OBJECT( pspec ) )
 		vips_object_clear_object( object, pspec );
 	else if( G_IS_PARAM_SPEC_BOXED( pspec ) ) {
 		gpointer *member = &G_STRUCT_MEMBER( gpointer, object,
 			argument_class->offset );
 
+#ifdef DEBUG
+		printf( "vips_object_dispose_argument: " );
+		vips_object_print_name( object );
+		printf( ".%s\n", g_param_spec_get_name( pspec ) ); 
+#endif /*DEBUG*/
+
 		if( *member ) {
 			g_boxed_free( G_PARAM_SPEC_VALUE_TYPE( pspec ),
 				*member );
@@ -429,6 +423,48 @@ vips_object_dispose_argument( VipsObject *object, GParamSpec *pspec,
 /* Free all args on this object which may be holding resources.
  */
 void
+vips_argument_dispose_all( VipsObject *object )
+{
+#ifdef DEBUG
+	printf( "vips_argument_dispose_all: " );
+	vips_object_print_name( object );
+	printf( "\n" );
+#endif /*DEBUG*/
+
+	vips_argument_map( object, vips_object_dispose_argument, NULL, NULL );
+}
+
+/* Free any args which are holding memory.
+ */
+static void *
+vips_object_free_argument( VipsObject *object, GParamSpec *pspec,
+	VipsArgumentClass *argument_class,
+	VipsArgumentInstance *argument_instance,
+	void *a, void *b )
+{
+	g_assert( ((VipsArgument *) argument_class)->pspec == pspec );
+	g_assert( ((VipsArgument *) argument_instance)->pspec == pspec );
+
+	if( G_IS_PARAM_SPEC_STRING( pspec ) ) {
+		char **member = &G_STRUCT_MEMBER( char *, object,
+			argument_class->offset );
+
+#ifdef DEBUG
+		printf( "vips_object_free_argument: " );
+		vips_object_print_name( object );
+		printf( ".%s\n", g_param_spec_get_name( pspec ) ); 
+#endif /*DEBUG*/
+
+		VIPS_FREE( *member );
+	}
+
+	return( NULL );
+}
+
+/* Free args which hold memory. Things like strings need to be freed right at
+ * the end in case anyone is still using them.
+ */
+static void
 vips_argument_free_all( VipsObject *object )
 {
 #ifdef DEBUG
@@ -437,7 +473,7 @@ vips_argument_free_all( VipsObject *object )
 	printf( "\n" );
 #endif /*DEBUG*/
 
-	vips_argument_map( object, vips_object_dispose_argument, NULL, NULL );
+	vips_argument_map( object, vips_object_free_argument, NULL, NULL );
 }
 
 static void
@@ -466,7 +502,7 @@ vips_object_dispose( GObject *gobject )
 
 	/* Clear all our arguments: they may be holding refs we should drop.
 	 */
-	vips_argument_free_all( object );
+	vips_argument_dispose_all( object );
 	VIPS_FREEF( vips_argument_table_destroy, object->argument_table );
 
 	vips_object_close( object );
@@ -474,6 +510,8 @@ vips_object_dispose( GObject *gobject )
 	G_OBJECT_CLASS( vips_object_parent_class )->dispose( gobject );
 
 	vips_object_postclose( object );
+
+	vips_argument_free_all( object );
 }
 
 static void

libvips/iofuncs/operation.c

@@ -359,7 +359,7 @@ vips_call( const char *operation_name, ... )
 	/* Build failed: junk args and back out.
 	 */
 	if( result ) {
-		vips_argument_free_all( VIPS_OBJECT( operation ) );
+		vips_argument_dispose_all( VIPS_OBJECT( operation ) );
 		g_object_unref( operation );
 
 		return( -1 );
@@ -406,7 +406,7 @@ vips_call_split( const char *operation_name, va_list optional, ... )
 	/* Build failed: junk args and back out.
 	 */
 	if( result ) {
-		vips_argument_free_all( VIPS_OBJECT( operation ) );
+		vips_argument_dispose_all( VIPS_OBJECT( operation ) );
 		g_object_unref( operation );
 
 		return( -1 );