From c81a12ee00e9f20717da09d2bd9b85ad310a6abe Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Sat, 3 May 2014 18:04:25 +0100
Subject: [PATCH] vips_system() now uses g_spawn_command_line_sync()

helps stop stray command windows appearing on Windows, better error msg
too
---
 ChangeLog                   |  1 +
 TODO                        |  5 ---
 libvips/include/vips/util.h |  2 ++
 libvips/iofuncs/system.c    | 65 +++++++++++++++++++++++++------------
 libvips/iofuncs/util.c      | 11 +++++++
 5 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 10c0ffc7..a1037bde 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -26,6 +26,7 @@
 - added vips_foreign_load_buffer(), vips_foreign_save_buffer()
 - added vips_object_set_from_string()
 - support 1/2/4 bit palette tiff images with alpha
+- vips_system() now uses g_spawn_command_line_sync()
 
 6/3/14 started 7.38.6
 - grey ramp minimum was wrong
diff --git a/TODO b/TODO
index 4b86a3b1..4a80ac3a 100644
--- a/TODO
+++ b/TODO
@@ -4,12 +4,7 @@
   vips_foreign_load_new_from_foreign_sub(), but it splits on ':' ... argh!
 
   deprecate this thing and stop ':' split
-
-
   
-
-- support 1/2/4 bit palette tiff with alpha
-
 - can we use postbuild elsewhere? look at use of "preclose" / "written", etc. 
 
 
diff --git a/libvips/include/vips/util.h b/libvips/include/vips/util.h
index 4bd3f069..dc559de4 100644
--- a/libvips/include/vips/util.h
+++ b/libvips/include/vips/util.h
@@ -186,6 +186,8 @@ gboolean vips_ispostfix( const char *a, const char *b );
 gboolean vips_isprefix( const char *a, const char *b );
 char *vips_break_token( char *str, const char *brk );
 
+void vips__chomp( char *str );
+
 int vips_vsnprintf( char *str, size_t size, const char *format, va_list ap );
 int vips_snprintf( char *str, size_t size, const char *format, ... )
 	__attribute__((format(printf, 3, 4)));
diff --git a/libvips/iofuncs/system.c b/libvips/iofuncs/system.c
index 335c5f8c..8c414b80 100644
--- a/libvips/iofuncs/system.c
+++ b/libvips/iofuncs/system.c
@@ -17,6 +17,9 @@
  * 4/6/13
  * 	- redo as a class
  * 	- input and output images are now optional
+ * 3/5/14
+ * 	- switch to g_spawn_command_line_sync() from popen() ... helps stop
+ * 	  stray command-windows on Windows
  */
 
 /*
@@ -119,8 +122,10 @@ vips_system_build( VipsObject *object )
 	char line[VIPS_PATH_MAX];
 	char txt[VIPS_PATH_MAX];
 	VipsBuf buf = VIPS_BUF_STATIC( txt );
-	char *p;
+	char *std_output;
+	char *std_error;
 	int result;
+	GError *error = NULL;
 
 	if( VIPS_OBJECT_CLASS( vips_system_parent_class )->build( object ) )
 		return( -1 );
@@ -165,30 +170,48 @@ vips_system_build( VipsObject *object )
 			cmd, VIPS_PATH_MAX, system->out_name ) )
 		return( -1 ); 
 
-	/* Swap all "%%" in the string for a single "%". We need this for
-	 * compatibility with older printf-based vips_system()s which
-	 * needed a double %%.
-	 */
-	for( p = cmd; *p; p++ ) 
-		if( p[0] == '%' && 
-			p[1] == '%' )
-			memmove( p, p + 1, strlen( p ) ); 
+	if( !g_spawn_command_line_sync( cmd, 
+		&std_output, &std_error, &result, &error ) ) {
+		if( error ) {
+			vips_error( class->nickname, "%s", error->message );
+			g_error_free( error );
+		}
+		if( std_error ) {
+			vips__chomp( std_error ); 
+			if( strcmp( std_error, "" ) != 0 )
+				vips_error( class->nickname, 
+					"error output: %s", std_error );
+			VIPS_FREE( std_error );
+		}
+		if( std_output ) {
+			vips__chomp( std_output ); 
+			if( strcmp( std_output, "" ) != 0 )
+				vips_error( class->nickname, 
+					"output: %s", std_output );
+			VIPS_FREE( std_output );
+		}
+		vips_error_system( result, class->nickname, 
+			"%s", _( "command failed" ) ); 
 
-	if( !(fp = vips_popenf( "%s", "r", cmd )) )
-		return( -1 );
-
-	while( fgets( line, VIPS_PATH_MAX, fp ) ) 
-		if( !vips_buf_appends( &buf, line ) )
-			break; 
-
-	g_object_set( system, "log", vips_buf_all( &buf ), NULL ); 
-
-	if( (result = pclose( fp )) ) {
-		vips_error( class->nickname, 
-			_( "command failed: \"%s\"" ), system->cmd_format );
 		return( -1 ); 
 	}
 
+	g_assert( !result ); 
+
+	if( std_error ) {
+		vips__chomp( std_error ); 
+		if( strcmp( std_error, "" ) != 0 )
+			vips_warn( class->nickname, 
+				_( "stderr output: %s" ), std_error ); 
+	}
+	if( std_output ) {
+		vips__chomp( std_output ); 
+		g_object_set( system, "log", std_output, NULL ); 
+	}
+
+	VIPS_FREE( std_output );
+	VIPS_FREE( std_error );
+
 	if( system->out_name ) {
 		VipsImage *out; 
 
diff --git a/libvips/iofuncs/util.c b/libvips/iofuncs/util.c
index c8840010..5a408152 100644
--- a/libvips/iofuncs/util.c
+++ b/libvips/iofuncs/util.c
@@ -1206,6 +1206,17 @@ vips_mkdirf( const char *name, ... )
         return( 0 );
 }
 
+/* Chop off any trailing whitespace.
+ */
+void
+vips__chomp( char *str )
+{
+	char *p;
+
+	for( p = str + strlen( str ); p > str && isspace( p[-1] ); p-- )
+		p[-1] = '\0';
+}
+
 /* Break a command-line argument into tokens separated by whitespace. 
  *
  * Strings can't be adjacent, so "hello world" (without quotes) is a single