fix deadlock with generate failing

and better error msg from libpng

TODO

@@ -9,10 +9,6 @@ blocking bugs
 
   similarly for vips_max() of an image which contains NaN, -INFINITY etc.
 
-- clicking on bad pngs in the file browser will lock nip2
-
-  they don't seem to bother "header" though, strange
-
 - new binding is still missing constants 
 
   how do boxed types work? confusing

libvips/foreign/vipspng.c

@@ -36,6 +36,8 @@
  * 7/2/12
  * 	- mild refactoring
  * 	- add support for sequential reads
+ * 23/2/12
+ * 	- add a longjmp() to our error handler to stop the default one running
  */
 
 /*
@@ -93,13 +95,18 @@
 static void
 user_error_function( png_structp png_ptr, png_const_charp error_msg )
 {
-	vips_error( "png2vips", _( "PNG error: \"%s\"" ), error_msg );
+	vips_error( "png2vips", "%s", error_msg );
+
+	/* This function must not return, or the default error handler will be
+	 * invoked.
+	 */
+	longjmp( png_jmpbuf( png_ptr ), -1 ); 
 }
 
 static void
 user_warning_function( png_structp png_ptr, png_const_charp warning_msg )
 {
-	vips_error( "png2vips", _( "PNG warning: \"%s\"" ), warning_msg );
+	vips_error( "png2vips", "%s", warning_msg );
 }
 
 /* What we track during a PNG read.
@@ -354,8 +361,13 @@ png2vips_generate( VipsRegion *or,
 	g_assert( r->width == or->im->Xsize );
 	g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize );
 
-	if( setjmp( png_jmpbuf( read->pPng ) ) ) 
+	if( setjmp( png_jmpbuf( read->pPng ) ) ) {
+#ifdef DEBUG
+		printf( "png2vips_generate: failing in setjmp\n" ); 
+#endif /*DEBUG*/
+
 		return( -1 );
+	}
 
 	for( y = 0; y < r->height; y++ ) {
 		png_bytep q = (png_bytep) VIPS_REGION_ADDR( or, 0, r->top + y );

libvips/iofuncs/sinkdisc.c

@@ -7,6 +7,8 @@
  * 	- better buffer handling for single-line images
  * 17/7/10
  * 	- we could get stuck if allocate failed (thanks Tim)
+ * 23/2/12
+ * 	- we could deadlock if generate failed
  */
 
 /*
@@ -404,21 +406,21 @@ wbuffer_work_fn( VipsThreadState *state, void *a )
 {
 	WriteThreadState *wstate = (WriteThreadState *) state;
 
+	int result;
+
 	VIPS_DEBUG_MSG( "wbuffer_work_fn: %p %d x %d\n", 
 		state, state->pos.left, state->pos.top );
 
-	if( vips_region_prepare_to( state->reg, wstate->buf->region, 
-		&state->pos, state->pos.left, state->pos.top ) ) {
-		VIPS_DEBUG_MSG( "wbuffer_work_fn: %p error!\n", state );
-		return( -1 );
-	}
-	VIPS_DEBUG_MSG( "wbuffer_work_fn: %p done\n", state );
+	result = vips_region_prepare_to( state->reg, wstate->buf->region, 
+		&state->pos, state->pos.left, state->pos.top );
+
+	VIPS_DEBUG_MSG( "wbuffer_work_fn: %p result = %d\n", state, result );
 
 	/* Tell the bg write thread we've left.
 	 */
 	vips_semaphore_upn( &wstate->buf->nwrite, 1 );
 
-	return( 0 );
+	return( result );
 }
 
 static void

libvips/iofuncs/sinkmemory.c

@@ -6,6 +6,8 @@
  * 
  * 17/2/12
  * 	- from sinkdisc.c
+ * 23/2/12
+ * 	- we could deadlock if generate failed
  */
 
 /*
@@ -250,22 +252,22 @@ sink_memory_area_work_fn( VipsThreadState *state, void *a )
 	SinkMemoryThreadState *wstate = (SinkMemoryThreadState *) state;
 	SinkMemoryArea *area = wstate->area;
 
+	int result;
+
 	VIPS_DEBUG_MSG( "sink_memory_area_work_fn: %p %d x %d\n", 
 		state, state->pos.left, state->pos.top );
 
-	if( vips_region_prepare_to( state->reg, memory->region, 
-		&state->pos, state->pos.left, state->pos.top ) ) {
-		VIPS_DEBUG_MSG( "sink_memory_area_work_fn: %p error!\n", 
-			state );
-		return( -1 );
-	}
-	VIPS_DEBUG_MSG( "sink_memory_area_work_fn: %p done\n", state );
+	result = vips_region_prepare_to( state->reg, memory->region, 
+		&state->pos, state->pos.left, state->pos.top );
+
+	VIPS_DEBUG_MSG( "sink_memory_area_work_fn: %p result = %d\n", 
+		state, result );
 
 	/* Tell the allocator we're done.
 	 */
 	vips_semaphore_upn( &area->nwrite, 1 );
 
-	return( 0 );
+	return( result );
 }
 
 static void