From 58b53506fffdaa024718c53b7e30087c580884eb Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 20 May 2022 16:38:04 +0100
Subject: [PATCH] add vips_target_end (#2802)

since finish did not return an error code

also make sure we don't call target_end from inside _dispose, since that
can't signal error either

see https://github.com/libvips/libvips/issues/2801
---
 ChangeLog                         |  1 +
 libvips/foreign/cgifsave.c        |  4 ++-
 libvips/foreign/csvsave.c         |  5 ++-
 libvips/foreign/heifsave.c        |  3 +-
 libvips/foreign/jp2ksave.c        |  3 +-
 libvips/foreign/jxlload.c         |  1 +
 libvips/foreign/jxlsave.c         |  8 +++--
 libvips/foreign/matrixsave.c      |  5 ++-
 libvips/foreign/pngsave.c         |  5 +--
 libvips/foreign/ppmsave.c         |  5 ++-
 libvips/foreign/radiance.c        |  3 +-
 libvips/foreign/spngsave.c        |  5 ++-
 libvips/foreign/vips2jpeg.c       |  3 +-
 libvips/foreign/vips2webp.c       |  3 +-
 libvips/foreign/vipspng.c         |  5 +--
 libvips/foreign/vipssave.c        |  5 +--
 libvips/include/vips/connection.h | 11 ++++--
 libvips/iofuncs/target.c          | 59 +++++++++++++++++++++++--------
 libvips/iofuncs/targetcustom.c    | 51 ++++++++++++++++++++++++--
 19 files changed, 140 insertions(+), 45 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8e34e283..6b506ec3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -21,6 +21,7 @@
 - add "gap" option to vips_reduce[hv]() and vips_resize() [kleisauke]
 - add "ceil" option to vips_shrink() [kleisauke]
 - quality improvements for image resizing [kleisauke]
+- add vips_target_end(), deprecate vips_target_finish()
 
 26/11/21 started 8.12.3
 - better arg checking for vips_hist_find_ndim() [travisbell]
diff --git a/libvips/foreign/cgifsave.c b/libvips/foreign/cgifsave.c
index 57325ef9..dc4dcf61 100644
--- a/libvips/foreign/cgifsave.c
+++ b/libvips/foreign/cgifsave.c
@@ -595,7 +595,9 @@ vips_foreign_save_cgif_build( VipsObject *object )
 		return( -1 );
 
 	VIPS_FREEF( cgif_close, cgif->cgif_context );
-	vips_target_finish( cgif->target );
+
+	if( vips_target_end( cgif->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/csvsave.c b/libvips/foreign/csvsave.c
index 076777ef..a1c13e57 100644
--- a/libvips/foreign/csvsave.c
+++ b/libvips/foreign/csvsave.c
@@ -69,8 +69,6 @@ vips_foreign_save_csv_dispose( GObject *gobject )
 {
 	VipsForeignSaveCsv *csv = (VipsForeignSaveCsv *) gobject;
 
-	if( csv->target ) 
-		vips_target_finish( csv->target );
 	VIPS_UNREF( csv->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_csv_parent_class )->
@@ -179,7 +177,8 @@ vips_foreign_save_csv_build( VipsObject *object )
 	if( vips_sink_disc( save->ready, vips_foreign_save_csv_block, csv ) )
 		return( -1 );
 
-	vips_target_finish( csv->target );
+	if( vips_target_end( csv->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/heifsave.c b/libvips/foreign/heifsave.c
index 333dabb6..c76842e3 100644
--- a/libvips/foreign/heifsave.c
+++ b/libvips/foreign/heifsave.c
@@ -558,7 +558,8 @@ vips_foreign_save_heif_build( VipsObject *object )
 		return( -1 );
 	}
 
-	vips_target_finish( heif->target );
+	if( vips_target_end( heif->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/jp2ksave.c b/libvips/foreign/jp2ksave.c
index ab7b7751..d21e45b8 100644
--- a/libvips/foreign/jp2ksave.c
+++ b/libvips/foreign/jp2ksave.c
@@ -902,7 +902,8 @@ vips_foreign_save_jp2k_build( VipsObject *object )
 
 	opj_end_compress( jp2k->codec, jp2k->stream );
 
-	vips_target_finish( jp2k->target );
+	if( vips_target_end( jp2k->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/jxlload.c b/libvips/foreign/jxlload.c
index 9e649667..b9cef077 100644
--- a/libvips/foreign/jxlload.c
+++ b/libvips/foreign/jxlload.c
@@ -131,6 +131,7 @@ vips_foreign_load_jxl_dispose( GObject *gobject )
 	VIPS_FREEF( JxlThreadParallelRunnerDestroy, jxl->runner );
 	VIPS_FREEF( JxlDecoderDestroy, jxl->decoder );
 	VIPS_FREE( jxl->icc_data );
+	VIPS_UNREF( jxl->source );
 
 	G_OBJECT_CLASS( vips_foreign_load_jxl_parent_class )->
 		dispose( gobject );
diff --git a/libvips/foreign/jxlsave.c b/libvips/foreign/jxlsave.c
index e20da32e..3d94b95c 100644
--- a/libvips/foreign/jxlsave.c
+++ b/libvips/foreign/jxlsave.c
@@ -31,6 +31,8 @@
 
  */
 
+#define DEBUG
+
 #ifdef HAVE_CONFIG_H
 #include <config.h>
 #endif /*HAVE_CONFIG_H*/
@@ -112,6 +114,7 @@ vips_foreign_save_jxl_dispose( GObject *gobject )
 
 	VIPS_FREEF( JxlThreadParallelRunnerDestroy, jxl->runner );
 	VIPS_FREEF( JxlEncoderDestroy, jxl->encoder );
+	VIPS_UNREF( jxl->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_jxl_parent_class )->
 		dispose( gobject );
@@ -168,6 +171,7 @@ static void
 vips_foreign_save_jxl_print_format( JxlPixelFormat *format )
 {
 	printf( "JxlPixelFormat:\n" );
+	printf( "    num_channels = %d\n", format->num_channels );
 	printf( "    data_type = " );
 	switch( format->data_type ) {
 	case JXL_TYPE_UINT8: 
@@ -191,7 +195,6 @@ vips_foreign_save_jxl_print_format( JxlPixelFormat *format )
 		break;
 	}
 	printf( "\n" );
-	printf( "    num_channels = %d\n", format->num_channels );
 	printf( "    endianness = %d\n", format->endianness );
 	printf( "    align = %zd\n", format->align );
 }
@@ -427,7 +430,8 @@ vips_foreign_save_jxl_build( VipsObject *object )
 		}
 	} while( status != JXL_ENC_SUCCESS );
 
-	vips_target_finish( jxl->target );
+	if( vips_target_end( jxl->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/matrixsave.c b/libvips/foreign/matrixsave.c
index 6055f302..30e9a729 100644
--- a/libvips/foreign/matrixsave.c
+++ b/libvips/foreign/matrixsave.c
@@ -69,8 +69,6 @@ vips_foreign_save_matrix_dispose( GObject *gobject )
 {
 	VipsForeignSaveMatrix *matrix = (VipsForeignSaveMatrix *) gobject;
 
-	if( matrix->target ) 
-		vips_target_finish( matrix->target );
 	VIPS_UNREF( matrix->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_matrix_parent_class )->
@@ -136,7 +134,8 @@ vips_foreign_save_matrix_build( VipsObject *object )
 		vips_foreign_save_matrix_block, matrix ) )
 		return( -1 );
 
-	vips_target_finish( matrix->target );
+	if( vips_target_end( matrix->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/pngsave.c b/libvips/foreign/pngsave.c
index 8375c4dd..a960c1c8 100644
--- a/libvips/foreign/pngsave.c
+++ b/libvips/foreign/pngsave.c
@@ -89,8 +89,6 @@ vips_foreign_save_png_dispose( GObject *gobject )
 {
 	VipsForeignSavePng *png = (VipsForeignSavePng *) gobject;
 
-	if( png->target ) 
-		vips_target_finish( png->target );
 	VIPS_UNREF( png->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_png_parent_class )->
@@ -159,6 +157,9 @@ vips_foreign_save_png_build( VipsObject *object )
 		return( -1 );
 	}
 
+	if( vips_target_end( png->target ) )
+		return( -1 );
+
 	g_object_unref( in );
 
 	return( 0 );
diff --git a/libvips/foreign/ppmsave.c b/libvips/foreign/ppmsave.c
index cb254d2b..b409bcb4 100644
--- a/libvips/foreign/ppmsave.c
+++ b/libvips/foreign/ppmsave.c
@@ -93,8 +93,6 @@ vips_foreign_save_ppm_dispose( GObject *gobject )
 {
 	VipsForeignSavePpm *ppm = (VipsForeignSavePpm *) gobject;
 
-	if( ppm->target ) 
-		vips_target_finish( ppm->target );
 	VIPS_UNREF( ppm->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_ppm_parent_class )->
@@ -423,7 +421,8 @@ vips_foreign_save_ppm_build( VipsObject *object )
 	if( vips_sink_disc( image, vips_foreign_save_ppm_block, ppm ) )
 		return( -1 );
 
-	vips_target_finish( ppm->target );
+	if( vips_target_end( ppm->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/radiance.c b/libvips/foreign/radiance.c
index 15165a36..8cdcee69 100644
--- a/libvips/foreign/radiance.c
+++ b/libvips/foreign/radiance.c
@@ -1185,7 +1185,8 @@ vips__rad_save( VipsImage *in, VipsTarget *target )
 		return( -1 );
 	}
 
-	vips_target_finish( target );
+	if( vips_target_end( target ) )
+		return( -1 );
 
 	write_destroy( write );
 
diff --git a/libvips/foreign/spngsave.c b/libvips/foreign/spngsave.c
index f5fd7cca..b2bf59f4 100644
--- a/libvips/foreign/spngsave.c
+++ b/libvips/foreign/spngsave.c
@@ -105,8 +105,6 @@ vips_foreign_save_spng_dispose( GObject *gobject )
 
 	GSList *p;
 
-	if( spng->target ) 
-		vips_target_finish( spng->target );
 	VIPS_UNREF( spng->target );
 	VIPS_UNREF( spng->memory );
 	VIPS_FREEF( spng_ctx_free, spng->ctx );
@@ -555,7 +553,8 @@ vips_foreign_save_spng_write( VipsForeignSaveSpng *spng, VipsImage *in )
 			return( -1 );
 	}
 
-	vips_target_finish( spng->target );
+	if( vips_target_end( spng->target ) )
+		return( -1 );
 
 	return( 0 );
 }
diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c
index 56e6617c..6fe1366f 100644
--- a/libvips/foreign/vips2jpeg.c
+++ b/libvips/foreign/vips2jpeg.c
@@ -825,7 +825,8 @@ term_destination( j_compress_ptr cinfo )
 		dest->buf, TARGET_BUFFER_SIZE - dest->pub.free_in_buffer ) )
 		ERREXIT( cinfo, JERR_FILE_WRITE );
 
-	vips_target_finish( dest->target );
+	if( vips_target_end( dest->target ) )
+		ERREXIT( cinfo, JERR_FILE_WRITE );
 }
 
 /* Set dest to one of our objects.
diff --git a/libvips/foreign/vips2webp.c b/libvips/foreign/vips2webp.c
index 9033528f..280fc8db 100644
--- a/libvips/foreign/vips2webp.c
+++ b/libvips/foreign/vips2webp.c
@@ -582,7 +582,8 @@ vips__webp_write_target( VipsImage *image, VipsTarget *target,
 		return( -1 );
 	}
 
-	vips_target_finish( target );
+	if( vips_target_end( target ) )
+		return( -1 );
 
 	vips_webp_write_unset( &write );
 
diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c
index 7fb65c00..f051de82 100644
--- a/libvips/foreign/vipspng.c
+++ b/libvips/foreign/vipspng.c
@@ -865,8 +865,6 @@ write_destroy( Write *write )
 #endif /*DEBUG*/
 
 	VIPS_UNREF( write->memory );
-	if( write->target ) 
-		vips_target_finish( write->target );
 	if( write->pPng )
 		png_destroy_write_struct( &write->pPng, &write->pInfo );
 	VIPS_FREE( write->row_pointer );
@@ -1306,6 +1304,9 @@ vips__png_write_target( VipsImage *in, VipsTarget *target,
 
 	write_destroy( write );
 
+	if( vips_target_end( target ) )
+		return( -1 );
+
 	return( 0 );
 }
 
diff --git a/libvips/foreign/vipssave.c b/libvips/foreign/vipssave.c
index 41fadc6f..ac3b8bed 100644
--- a/libvips/foreign/vipssave.c
+++ b/libvips/foreign/vipssave.c
@@ -65,8 +65,6 @@ vips_foreign_save_vips_dispose( GObject *gobject )
 {
 	VipsForeignSaveVips *vips = (VipsForeignSaveVips *) gobject;
 
-	if( vips->target ) 
-		vips_target_finish( vips->target );
 	VIPS_UNREF( vips->target );
 
 	G_OBJECT_CLASS( vips_foreign_save_vips_parent_class )->
@@ -116,6 +114,9 @@ vips_foreign_save_vips_build( VipsObject *object )
 		return( -1 );
 	}
 
+	if( vips_target_end( vips->target ) )
+		return( -1 );
+
 	return( 0 );
 }
 
diff --git a/libvips/include/vips/connection.h b/libvips/include/vips/connection.h
index eaafabc1..b0157aaa 100644
--- a/libvips/include/vips/connection.h
+++ b/libvips/include/vips/connection.h
@@ -406,9 +406,9 @@ struct _VipsTarget {
 	 */
 	gboolean memory;
 
-	/* The target has been finished and can no longer be written.
+	/* The target has been ended and can no longer be written.
 	 */
-	gboolean finished;
+	gboolean ended;
 
 	/* Write memory output here.
 	 */
@@ -439,6 +439,10 @@ typedef struct _VipsTargetClass {
 	/* Output has been generated, so do any clearing up,
 	 * eg. copy the bytes we saved in memory to the target blob.
 	 */
+	int (*end)( VipsTarget * );
+
+	/* Deprecated in favour of ::end.
+	 */
 	void (*finish)( VipsTarget * );
 
 } VipsTargetClass;
@@ -455,6 +459,8 @@ VipsTarget *vips_target_new_to_memory( void );
 VIPS_API
 int vips_target_write( VipsTarget *target, const void *data, size_t length );
 VIPS_API
+int vips_target_end( VipsTarget *target );
+VIPS_DEPRECATED_FOR(vips_target_end)
 void vips_target_finish( VipsTarget *target );
 VIPS_API
 unsigned char *vips_target_steal( VipsTarget *target, size_t *length );
@@ -508,6 +514,7 @@ typedef struct _VipsTargetCustomClass {
 	 */
 
 	gint64 (*write)( VipsTargetCustom *, const void *, gint64 );
+	int (*end)( VipsTargetCustom * );
 	void (*finish)( VipsTargetCustom * );
 
 } VipsTargetCustomClass;
diff --git a/libvips/iofuncs/target.c b/libvips/iofuncs/target.c
index f5a05976..91156667 100644
--- a/libvips/iofuncs/target.c
+++ b/libvips/iofuncs/target.c
@@ -148,6 +148,14 @@ vips_target_write_real( VipsTarget *target, const void *data, size_t length )
 	return( write( connection->descriptor, data, length ) );
 }
 
+static int
+vips_target_end_real( VipsTarget *target ) 
+{
+	VIPS_DEBUG_MSG( "vips_target_finish_real:\n" );
+
+	return( 0 );
+}
+
 static void
 vips_target_finish_real( VipsTarget *target ) 
 {
@@ -170,6 +178,7 @@ vips_target_class_init( VipsTargetClass *class )
 	object_class->build = vips_target_build;
 
 	class->write = vips_target_write_real;
+	class->end = vips_target_end_real;
 	class->finish = vips_target_finish_real;
 
 	VIPS_ARG_BOOL( class, "memory", 3, 
@@ -294,7 +303,7 @@ vips_target_write_unbuffered( VipsTarget *target,
 
 	VIPS_DEBUG_MSG( "vips_target_write_unbuffered:\n" );
 
-	if( target->finished )
+	if( target->ended )
 		return( 0 );
 
 	if( target->memory_buffer ) 
@@ -376,7 +385,7 @@ vips_target_write( VipsTarget *target, const void *buffer, size_t length )
 }
 
 /**
- * vips_target_finish:
+ * vips_target_end:
  * @target: target to operate on
  * @buffer: bytes to write
  * @length: length of @buffer in bytes
@@ -385,18 +394,21 @@ vips_target_write( VipsTarget *target, const void *buffer, size_t length )
  * can call it many times. 
  *
  * After a target has been finished, further writes will do nothing.
+ *
+ * Returns: 0 on success, -1 on error.
  */
-void
-vips_target_finish( VipsTarget *target )
+int
+vips_target_end( VipsTarget *target )
 {
 	VipsTargetClass *class = VIPS_TARGET_GET_CLASS( target );
 
-	VIPS_DEBUG_MSG( "vips_target_finish:\n" );
+	VIPS_DEBUG_MSG( "vips_target_end:\n" );
 
-	if( target->finished )
-		return;
+	if( target->ended )
+		return( 0 );
 
-	(void) vips_target_flush( target );
+	if( vips_target_flush( target ) )
+		return( -1 );
 
 	/* Move the target buffer into the blob so it can be read out.
 	 */
@@ -410,10 +422,28 @@ vips_target_finish( VipsTarget *target )
 		vips_blob_set( target->blob,
 			(VipsCallbackFn) vips_area_free_cb, data, length );
 	}
-	else
-		class->finish( target );
+	else {
+		if( class->end( target ) )
+			return( -1 );
+	}
 
-	target->finished = TRUE;
+	target->ended = TRUE;
+
+	return( 0 );
+}
+
+/**
+ * vips_target_finish:
+ * @target: target to operate on
+ * @buffer: bytes to write
+ * @length: length of @buffer in bytes
+ *
+ * Deprecated in favour of vips_target_end().
+ */
+void
+vips_target_finish( VipsTarget *target )
+{
+	(void) vips_target_end( target );
 }
 
 /**
@@ -440,7 +470,7 @@ vips_target_steal( VipsTarget *target, size_t *length )
 	(void) vips_target_flush( target );
 
 	if( !target->memory_buffer ||
-		target->finished ) {
+		target->ended ) {
 		if( length )
 			*length = target->memory_buffer->len;
 
@@ -452,11 +482,12 @@ vips_target_steal( VipsTarget *target, size_t *length )
 	data = g_byte_array_free( target->memory_buffer, FALSE );
 	target->memory_buffer = NULL;
 
-	/* We must have a valid byte array or finish will fail.
+	/* We must have a valid byte array or end will fail.
 	 */
 	target->memory_buffer = g_byte_array_new();
 
-	vips_target_finish( target );
+	if( vips_target_end( target ) )
+		return( NULL );
 
 	return( data );
 }
diff --git a/libvips/iofuncs/targetcustom.c b/libvips/iofuncs/targetcustom.c
index f5b6df20..90c76732 100644
--- a/libvips/iofuncs/targetcustom.c
+++ b/libvips/iofuncs/targetcustom.c
@@ -64,6 +64,7 @@ G_DEFINE_TYPE( VipsTargetCustom, vips_target_custom, VIPS_TYPE_TARGET );
  */
 enum {
 	SIG_WRITE,		
+	SIG_END,		
 	SIG_FINISH,		
 	SIG_LAST
 };
@@ -90,10 +91,27 @@ vips_target_custom_write_real( VipsTarget *target,
 	return( bytes_written );
 }
 
+static int
+vips_target_custom_end_real( VipsTarget *target )
+{
+	int result;
+
+	VIPS_DEBUG_MSG( "vips_target_custom_end_real:\n" );
+
+	/* Return value if no attached handler.
+	 */
+	result = 0;
+
+	g_signal_emit( target, vips_target_custom_signals[SIG_END], 0,
+		&result ); 
+
+	return( result );
+}
+
 static void
 vips_target_custom_finish_real( VipsTarget *target )
 {
-	VIPS_DEBUG_MSG( "vips_target_custom_seek_real:\n" );
+	VIPS_DEBUG_MSG( "vips_target_custom_finish_real:\n" );
 
 	g_signal_emit( target, vips_target_custom_signals[SIG_FINISH], 0 );
 }
@@ -107,6 +125,14 @@ vips_target_custom_write_signal_real( VipsTargetCustom *target_custom,
 	return( 0 );
 }
 
+static int
+vips_target_custom_end_signal_real( VipsTargetCustom *target_custom ) 
+{
+	VIPS_DEBUG_MSG( "vips_target_custom_end_signal_real:\n" );
+
+	return( 0 );
+}
+
 static void
 vips_target_custom_finish_signal_real( VipsTargetCustom *target_custom ) 
 {
@@ -123,9 +149,11 @@ vips_target_custom_class_init( VipsTargetCustomClass *class )
 	object_class->description = _( "Custom target" );
 
 	target_class->write = vips_target_custom_write_real;
+	target_class->end = vips_target_custom_end_real;
 	target_class->finish = vips_target_custom_finish_real;
 
 	class->write = vips_target_custom_write_signal_real;
+	class->end = vips_target_custom_end_signal_real;
 	class->finish = vips_target_custom_finish_signal_real;
 
 	/**
@@ -148,11 +176,27 @@ vips_target_custom_class_init( VipsTargetCustomClass *class )
 		G_TYPE_POINTER, G_TYPE_INT64 );
 
 	/**
-	 * VipsTargetCustom::finish:
+	 * VipsTargetCustom::end:
 	 * @target_custom: the target being operated on
 	 *
 	 * This signal is emitted at the end of write. The target should do
 	 * any finishing necessary.
+	 *
+	 * Returns: 0 on success, -1 on error.
+	 */
+	vips_target_custom_signals[SIG_END] = g_signal_new( "end",
+		G_TYPE_FROM_CLASS( class ),
+		G_SIGNAL_ACTION,
+		G_STRUCT_OFFSET( VipsTargetCustomClass, end ), 
+		NULL, NULL,
+		vips_INT__VOID,
+		G_TYPE_NONE, 0 );
+
+	/**
+	 * VipsTargetCustom::finish:
+	 * @target_custom: the target being operated on
+	 *
+	 * Deprecated for VipsTargetCustom::end.
 	 */
 	vips_target_custom_signals[SIG_FINISH] = g_signal_new( "finish",
 		G_TYPE_FROM_CLASS( class ),
@@ -183,7 +227,8 @@ vips_target_custom_new( void )
 
 	VIPS_DEBUG_MSG( "vips_target_custom_new:\n" );
 
-	target_custom = VIPS_TARGET_CUSTOM( g_object_new( VIPS_TYPE_TARGET_CUSTOM, NULL ) );
+	target_custom = VIPS_TARGET_CUSTOM( g_object_new( 
+		VIPS_TYPE_TARGET_CUSTOM, NULL ) );
 
 	if( vips_object_build( VIPS_OBJECT( target_custom ) ) ) {
 		VIPS_UNREF( target_custom );