From f2a178e98f164cde38f5295668e1a8f6a0550033 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Tue, 28 Feb 2017 13:40:34 +0000
Subject: [PATCH] move buf writers on top of dbuf

tiff and webp not moved
---
 TODO                        |  22 -----
 libvips/foreign/radiance.c  | 134 +++++++--------------------
 libvips/foreign/vips2jpeg.c |   8 +-
 libvips/include/vips/dbuf.h |  14 ++-
 libvips/iofuncs/dbuf.c      | 179 ++++++++++++++++++++++++++++--------
 libvips/iofuncs/vips.c      |   6 +-
 6 files changed, 196 insertions(+), 167 deletions(-)

diff --git a/TODO b/TODO
index ed0c97b8..e50960d6 100644
--- a/TODO
+++ b/TODO
@@ -1,25 +1,3 @@
-- use VipsDbuf for tiffsave_buffer etc.
-
-	$ grep -l save_buffer *.c
-	dzsave.c
-	jpegsave.c
-	pngsave.c
-	radsave.c
-	tiffsave.c
-	webpsave.c
-
-  done png, jpg
-
-  tiff needs seek ... perhaps add this to dbuf?
-
-  dzsave uses gsf
-
-  rad and webp left to do
-
-  vips2jpeg term_destination() needs some kind of truncate call for dbuf
-
-
-
 - verify xml data against master for vips save and dzsave
 
 
diff --git a/libvips/foreign/radiance.c b/libvips/foreign/radiance.c
index d6cddeb2..69e80f59 100644
--- a/libvips/foreign/radiance.c
+++ b/libvips/foreign/radiance.c
@@ -17,6 +17,8 @@
  * 	  readers
  * 23/5/16
  *	- add buffer save functions   
+ * 28/2/17
+ * 	- use dbuf for buffer output
  */
 
 /*
@@ -1172,9 +1174,7 @@ typedef struct {
 	char *filename;
 	FILE *fout;
 
-	char *buf;
-	size_t len;
-	size_t alloc;
+	VipsDbuf dbuf; 
 
 	char format[256];
 	double expos;
@@ -1189,7 +1189,7 @@ write_destroy( Write *write )
 {
 	VIPS_FREE( write->filename );
 	VIPS_FREEF( fclose, write->fout );
-	VIPS_FREE( write->buf );
+	vips_dbuf_destroy( &write->dbuf );
 
 	vips_free( write );
 }
@@ -1208,9 +1208,7 @@ write_new( VipsImage *in )
 	write->filename = NULL;
 	write->fout = NULL;
 
-	write->buf = NULL;
-	write->len = 0;
-	write->alloc = 0;
+	vips_dbuf_init( &write->dbuf ); 
 
 	strcpy( write->format, COLRFMT );
 	write->expos = 1.0;
@@ -1346,91 +1344,30 @@ vips__rad_save( VipsImage *in, const char *filename )
 	return( 0 );
 }
 
-static void
-write_buf_grow( Write *write, size_t grow_len )
-{
-	size_t new_len = write->len + grow_len;
-
-	if( new_len > write->alloc ) {
-		size_t proposed_alloc = (16 + write->alloc) * 3 / 2;
-
-		write->alloc = VIPS_MAX( proposed_alloc, new_len );
-
-		/* Our caller must free with g_free(), so we must use
-		 * g_realloc(). 
-		 */
-		write->buf = g_realloc( write->buf, write->alloc );
-
-		VIPS_DEBUG_MSG( "write_buf_grow: grown to %zd bytes\n",
-			write->alloc );
-	}
-}
-
-static void
-bprintf( Write *write, const char *fmt, ... )
-{
-	int length;
-	char *write_start;
-	va_list ap;
-
-	/* Determine required size.
-	 */
-	va_start( ap, fmt );
-	length = vsnprintf( NULL, 0, fmt, ap );
-	va_end( ap );
-
-	write_buf_grow( write, length + 1 );
-
-	write_start = write->buf + write->len;
-
-	va_start( ap, fmt );
-	length = vsnprintf( write_start, length + 1, fmt, ap );
-	va_end( ap );
-
-	write->len += length;
-
-	g_assert( write->len <= write->alloc );
-}
-
-#define bputformat( write, s ) \
-	bprintf( write, "%s%s\n", FMTSTR, s )
-
-#define bputexpos( write, ex ) \
-	bprintf( write, "%s%e\n", EXPOSSTR, ex )
-
-#define bputcolcor( write, cc ) \
-	bprintf( write, "%s %f %f %f\n", \
-		COLCORSTR, (cc)[RED], (cc)[GRN], (cc)[BLU] )
-
-#define bputaspect( write, pa ) \
-	bprintf( write, "%s%f\n", ASPECTSTR, pa )
-
-#define bputprims( write, p ) \
-	bprintf( write, "%s %.4f %.4f %.4f %.4f %.4f %.4f %.4f %.4f\n", \
-		PRIMARYSTR, \
-		(p)[RED][CIEX], (p)[RED][CIEY], \
-		(p)[GRN][CIEX], (p)[GRN][CIEY], \
-		(p)[BLU][CIEX], (p)[BLU][CIEY], \
-		(p)[WHT][CIEX], (p)[WHT][CIEY] )
-
-#define bputsresolu( write, rs ) \
-	bprintf( write, "%s", resolu2str( resolu_buf, rs ) )
-
 static int
 vips2rad_put_header_buf( Write *write )
 {
 	vips2rad_make_header( write );
 
-	bprintf( write, "#?RADIANCE\n" );
-
-	bputformat( write, write->format );
-	bputexpos( write, write->expos );
-	bputcolcor( write, write->colcor );
-	bprintf( write, "SOFTWARE=vips %s\n", vips_version_string() );
-	bputaspect( write, write->aspect );
-	bputprims( write, write->prims );
-	bprintf( write, "\n" );
-	bputsresolu( write, &write->rs );
+	vips_dbuf_appendf( &write->dbuf, "#?RADIANCE\n" );
+	vips_dbuf_appendf( &write->dbuf, "%s%s\n", FMTSTR, write->format );
+	vips_dbuf_appendf( &write->dbuf, "%s%e\n", EXPOSSTR, write->expos );
+	vips_dbuf_appendf( &write->dbuf, "%s %f %f %f\n", 
+		COLCORSTR, 
+		write->colcor[RED], write->colcor[GRN], write->colcor[BLU] );
+	vips_dbuf_appendf( &write->dbuf, "SOFTWARE=vips %s\n", 
+		vips_version_string() );
+	vips_dbuf_appendf( &write->dbuf, "%s%f\n", ASPECTSTR, write->aspect );
+	vips_dbuf_appendf( &write->dbuf, 
+		"%s %.4f %.4f %.4f %.4f %.4f %.4f %.4f %.4f\n", 
+		PRIMARYSTR, 
+		write->prims[RED][CIEX], write->prims[RED][CIEY], 
+		write->prims[GRN][CIEX], write->prims[GRN][CIEY], 
+		write->prims[BLU][CIEX], write->prims[BLU][CIEY], 
+		write->prims[WHT][CIEX], write->prims[WHT][CIEY] );
+	vips_dbuf_appendf( &write->dbuf, "\n" );
+	vips_dbuf_appendf( &write->dbuf, "%s", 
+		resolu2str( resolu_buf, &write->rs ) );
 
 	return( 0 );
 }
@@ -1441,25 +1378,25 @@ static int
 scanline_write_buf( Write *write, COLR *scanline, int width )
 {
 	unsigned char *buffer;
+	size_t size;
+	int length;
 
-	write_buf_grow( write, MAX_LINE );
-	buffer = (unsigned char *) write->buf + write->len;
+	vips_dbuf_allocate( &write->dbuf, MAX_LINE );
+	buffer = vips_dbuf_get_write( &write->dbuf, &size );
 
 	if( width < MINELEN || 
 		width > MAXELEN ) {
 		/* Write as a flat scanline.
 		 */
-		memcpy( buffer, scanline, sizeof( COLR ) * width );
-		write->len += sizeof( COLR ) * width;
+		length = sizeof( COLR ) * width;
+		memcpy( buffer, scanline, length ); 
 	}
-	else {
-		int length;
-
+	else 
 		/* An RLE scanline.
 		 */
 		rle_scanline_write( scanline, width, buffer, &length );
-		write->len += length;
-	}
+
+	vips_dbuf_seek( &write->dbuf, length - size, SEEK_CUR ); 
 
 	return( 0 );
 }
@@ -1510,10 +1447,7 @@ vips__rad_save_buf( VipsImage *in, void **obuf, size_t *olen )
 		return( -1 );
 	}
 
-	*obuf = write->buf;
-	write->buf = NULL;
-	if( olen )
-		*olen = write->len;
+	*obuf = vips_dbuf_steal( &write->dbuf, olen );
 
 	write_destroy( write );
 
diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c
index 0f8f0365..1c878986 100644
--- a/libvips/foreign/vips2jpeg.c
+++ b/libvips/foreign/vips2jpeg.c
@@ -734,10 +734,12 @@ term_destination( j_compress_ptr cinfo )
 
 	size_t size;
 
-	/* This can be called before the output area fills. We need to chop
-	 * size down to the number of bytes actually written.
+	/* We probably won't have filled the area that was last allocated in 
+	 * empty_output_buffer(). Chop the data size down to the length that
+	 * was actually written.
 	 */
-	buf->dbuf.write_point = buf->dbuf.max_size - buf->pub.free_in_buffer;
+	vips_dbuf_seek( &buf->dbuf, -buf->pub.free_in_buffer, SEEK_END );
+	vips_dbuf_truncate( &buf->dbuf ); 
 
 	*(buf->obuf) = vips_dbuf_steal( &buf->dbuf, &size );
 	*(buf->olen) = size; 
diff --git a/libvips/include/vips/dbuf.h b/libvips/include/vips/dbuf.h
index a23fc44d..58cb5f47 100644
--- a/libvips/include/vips/dbuf.h
+++ b/libvips/include/vips/dbuf.h
@@ -49,11 +49,17 @@ typedef struct _VipsDbuf {
 	/* The current base, and the size of the allocated memory area.
 	 */
 	unsigned char *data;
-	size_t max_size;
+	size_t allocated_size;
 
-	/* And the write point.
+	/* The size of the actual data that's been written. This will usually
+	 * be <= allocated_size, but always >= write_point.
+	 */
+	size_t data_size;
+
+	/* The write point.
 	 */
 	size_t write_point;
+
 } VipsDbuf; 
 
 void vips_dbuf_destroy( VipsDbuf *buf );
@@ -63,8 +69,10 @@ unsigned char *vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size );
 gboolean vips_dbuf_append( VipsDbuf *dbuf, 
 	const unsigned char *data, size_t size );
 gboolean vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... );
-void vips_dbuf_rewind( VipsDbuf *dbuf );
+void vips_dbuf_reset( VipsDbuf *dbuf );
 void vips_dbuf_destroy( VipsDbuf *dbuf );
+gboolean vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence );
+void vips_dbuf_truncate( VipsDbuf *dbuf );
 unsigned char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size );
 unsigned char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size );
 
diff --git a/libvips/iofuncs/dbuf.c b/libvips/iofuncs/dbuf.c
index 823027fe..f1b7b0e9 100644
--- a/libvips/iofuncs/dbuf.c
+++ b/libvips/iofuncs/dbuf.c
@@ -51,37 +51,74 @@ void
 vips_dbuf_init( VipsDbuf *dbuf )
 {
 	dbuf->data = NULL;
-	dbuf->max_size = 0;
+	dbuf->allocated_size = 0;
+	dbuf->data_size = 0;
 	dbuf->write_point = 0;
 }
 
+/**
+ * vips_dbuf_minimum_size:
+ * @dbuf: the buffer
+ * @size: the minimum size 
+ *
+ * Make sure @dbuf is at least @size bytes.
+ * 
+ * Returns: %FALSE on out of memory, %TRUE otherwise.
+ */
+gboolean
+vips_dbuf_minimum_size( VipsDbuf *dbuf, size_t size )
+{
+	if( size > dbuf->allocated_size ) { 
+		size_t new_allocated_size = 3 * (16 + size) / 2;
+
+		unsigned char *new_data;
+
+		if( !(new_data = 
+			g_try_realloc( dbuf->data, new_allocated_size )) ) {
+			vips_error( "VipsDbuf", "%s", _( "out of memory" ) );
+			return( FALSE );
+		}
+
+		dbuf->data = new_data;
+		dbuf->allocated_size = new_allocated_size;
+	}
+
+	return( TRUE ); 
+}
+
 /**
  * vips_dbuf_allocate:
  * @dbuf: the buffer
  * @size: the size to allocate
  *
- * Make sure @dbuf has at least @size bytes available for writing.
+ * Make sure @dbuf has at least @size bytes available after the write point.
  * 
  * Returns: %FALSE on out of memory, %TRUE otherwise.
  */
 gboolean
 vips_dbuf_allocate( VipsDbuf *dbuf, size_t size )
 {
-	size_t new_write_point = dbuf->write_point + size;
+	return( vips_dbuf_minimum_size( dbuf, dbuf->write_point + size ) ); 
+}
 
-	if( new_write_point > dbuf->max_size ) { 
-		size_t new_max_size = 3 * (16 + new_write_point) / 2;
+/**
+ * vips_dbuf_null_terminate:
+ * @dbuf: the buffer
+ *
+ * Make sure the byte after the last data byte is `\0`. This extra byte is not
+ * included in the data size and the write point is not moved.
+ *
+ * This makes it safe to treat the dbuf contents as a C string. 
+ * 
+ * Returns: %FALSE on out of memory, %TRUE otherwise.
+ */
+static gboolean
+vips_dbuf_null_terminate( VipsDbuf *dbuf )
+{
+	if( !vips_dbuf_minimum_size( dbuf, dbuf->data_size + 1 ) )
+		return( FALSE );
 
-		unsigned char *new_data;
-
-		if( !(new_data = g_try_realloc( dbuf->data, new_max_size )) ) {
-			vips_error( "VipsDbuf", "%s", _( "out of memory" ) );
-			return( FALSE );
-		}
-
-		dbuf->data = new_data;
-		dbuf->max_size = new_max_size;
-	}
+	dbuf->data[dbuf->data_size] = 0;
 
 	return( TRUE ); 
 }
@@ -92,21 +129,28 @@ vips_dbuf_allocate( VipsDbuf *dbuf, size_t size )
  * @size: (allow-none): optionally return length in bytes here
  *
  * Return a pointer to an area you can write to, return length of area in
- * @size. Use vips_dbuf_allocate() before this call to make the space.
+ * @size. Use vips_dbuf_allocate() before this call to set a minimum amount of
+ * space to have available. 
+ *
+ * The write point moves to just beyond the returned block. Use
+ * vips_dbuf_seek() to move it back again.
  * 
  * Returns: (transfer none): start of write area.
  */
 unsigned char *
 vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size )
 {
-	unsigned char *data = dbuf->data + dbuf->write_point;
+	unsigned char *write = dbuf->data + dbuf->write_point;
+	size_t length = dbuf->data + dbuf->allocated_size - write;
+
+	memset( write, 0, length ); 
+	dbuf->write_point = dbuf->allocated_size;
+	dbuf->data_size = dbuf->allocated_size;
 
 	if( size )
-		*size = dbuf->max_size - dbuf->write_point;
+		*size = length;
 
-	dbuf->write_point = dbuf->max_size;
-
-	return( data ); 
+	return( write ); 
 }
 
 /**
@@ -115,7 +159,7 @@ vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size )
  * @data: the data to append to the buffer
  * @size: the size of the len to append
  *
- * Append len bytes from @data to the buffer. The buffer expands if necessary. 
+ * Append @size bytes from @data. @dbuf expands if necessary. 
  * 
  * Returns: %FALSE on out of memory, %TRUE otherwise.
  */
@@ -127,6 +171,7 @@ vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size )
 
 	memcpy( dbuf->data + dbuf->write_point, data, size );
 	dbuf->write_point += size;
+	dbuf->data_size = VIPS_MAX( dbuf->data_size, dbuf->write_point );
 
 	return( TRUE ); 
 }
@@ -161,16 +206,17 @@ vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... )
 }
 
 /**
- * vips_dbuf_rewind:
+ * vips_dbuf_reset:
  * @dbuf: the buffer
  *
- * Reset the buffer to empty. No memory is freed, just the write pointer is
- * reset.
+ * Reset the buffer to empty. No memory is freed, just the data size and
+ * write point are reset.
  */
 void
-vips_dbuf_rewind( VipsDbuf *dbuf )
+vips_dbuf_reset( VipsDbuf *dbuf )
 {
 	dbuf->write_point = 0;
+	dbuf->data_size = 0;
 }
 
 /**
@@ -182,9 +228,73 @@ vips_dbuf_rewind( VipsDbuf *dbuf )
 void
 vips_dbuf_destroy( VipsDbuf *dbuf )
 {
+	vips_dbuf_reset( dbuf ); 
+
 	VIPS_FREE( dbuf->data ); 
-	dbuf->max_size = 0;
-	dbuf->write_point = 0;
+	dbuf->allocated_size = 0;
+}
+
+/**
+ * vips_dbuf_seek:
+ * @dbuf: the buffer
+ * @offset: how to move the write point
+ * @whence: from start, from end, from current
+ *
+ * Move the write point. @whence can be %SEEK_SET, %SEEK_CUR, %SEEK_END, with 
+ * the usual meaning. 
+ */
+gboolean
+vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence )
+{
+	off_t new_write_point;
+
+	switch( whence ) {
+	case SEEK_SET:
+		new_write_point = offset; 
+		break;
+
+	case SEEK_END:
+		new_write_point = dbuf->data_size + offset; 
+		break;
+
+	case SEEK_CUR:
+		new_write_point = dbuf->write_point + offset; 
+		break;
+
+	default:
+		g_assert( 0 ); 
+		new_write_point = dbuf->write_point; 
+		break;
+	}
+
+	if( new_write_point < 0 ) {
+		vips_error( "VipsDbuf", "%s", "negative seek" ); 
+		return( FALSE );
+	}
+
+	/* Possibly need to grow the buffer
+	 */
+	if( !vips_dbuf_minimum_size( dbuf, new_write_point ) )
+		return( FALSE ); 
+	dbuf->write_point = new_write_point; 
+	if( dbuf->data_size < dbuf->write_point ) { 
+		memset( dbuf->data, 0, dbuf->write_point - dbuf->data_size ); 
+		dbuf->data_size = dbuf->write_point;
+	}
+
+	return( TRUE ); 
+}
+
+/**
+ * vips_dbuf_truncate:
+ * @dbuf: the buffer
+ *
+ * Truncate the data so that it ends at the write point. No memory is freed. 
+ */
+void
+vips_dbuf_truncate( VipsDbuf *dbuf )
+{
+	dbuf->data_size = dbuf->write_point;
 }
 
 /**
@@ -205,17 +315,15 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size )
 {
 	unsigned char *data;
 
-	vips_dbuf_append( dbuf, (unsigned char *) "", 1 );
-	dbuf->write_point -= 1;
+	vips_dbuf_null_terminate( dbuf ); 
 
 	data = dbuf->data;
 
 	if( size )
-		*size = dbuf->write_point;
+		*size = dbuf->data_size;
 
 	dbuf->data = NULL;
-	dbuf->max_size = 0;
-	dbuf->write_point = 0;
+	vips_dbuf_destroy( dbuf );
 
 	return( data ); 
 }
@@ -235,11 +343,10 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size )
 unsigned char *
 vips_dbuf_string( VipsDbuf *dbuf, size_t *size )
 {
-	vips_dbuf_append( dbuf, (unsigned char *) "", 1 );
-	dbuf->write_point -= 1;
+	vips_dbuf_null_terminate( dbuf ); 
 
 	if( size )
-		*size = dbuf->write_point;
+		*size = dbuf->data_size;
 
 	return( dbuf->data ); 
 }
diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c
index e33a47ab..9b4c3935 100644
--- a/libvips/iofuncs/vips.c
+++ b/libvips/iofuncs/vips.c
@@ -536,7 +536,7 @@ parser_element_start_handler( void *user_data,
 				vips_strncpy( vep->type, p[1], MAX_PARSE_ATTR ); 
 		}
 
-		vips_dbuf_rewind( &vep->dbuf );
+		vips_dbuf_reset( &vep->dbuf );
 	} 
 	else if( strcmp( name, "header" ) == 0 )  
 		vep->header = TRUE;
@@ -545,7 +545,7 @@ parser_element_start_handler( void *user_data,
 	else if( strcmp( name, "root" ) == 0 ) {
 		for( p = atts; *p; p += 2 ) 
 			if( strcmp( p[0], "xmlns" ) == 0 &&
-				!vips_isprefix( NAMESPACE_URI "/vips", p[1] ) ) {
+				!vips_isprefix( NAMESPACE_URI "vips", p[1] ) ) {
 				vips_error( "VipsImage", "%s", 
 					_( "incorrect namespace in XML" ) );
 				vep->error = TRUE;
@@ -812,7 +812,7 @@ build_xml( VipsImage *image )
 	vips_dbuf_init( &dbuf ); 
 
 	vips_dbuf_appendf( &dbuf, "<?xml version=\"1.0\"?>\n" ); 
-	vips_dbuf_appendf( &dbuf, "<root xmlns=\"%s/vips/%d.%d.%d\">\n", 
+	vips_dbuf_appendf( &dbuf, "<root xmlns=\"%svips/%d.%d.%d\">\n", 
 		NAMESPACE_URI, 
 		VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION );
 	vips_dbuf_appendf( &dbuf, "  <header>\n" );