[m-dev.] diff: MR_make_string improvement

[Peter Ross]

On Thu, Aug 17, 2000 at 02:33:39AM +1000, Fergus Henderson wrote:
> On 16-Aug-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> > Estimated hours taken: 0.5
> > 
> > runtime/mercury_string.c:
> > 	Modify MR_make_string so that it first tries printing into a
> > 	fixed size buffer.  Only if that buffer is not big enough do we
> > 	allocate a buffer on the heap.
> 
> That looks OK, but there's room for improvement:
> 
> > +#define SIZE	4096
> 
> It would be better to use a more descriptive name than `SIZE'.
> 
> >  MR_String
> >  MR_make_string(MR_Code *proclabel, const char *fmt, ...) {
> >  	va_list		ap;
> >  	MR_String	result;
> >  	int 		n;
> > +	char		*p;
> >  
> >  #if defined(HAVE_VSNPRINTF) || defined(HAVE__VSNPRINTF)
> > +	int 		size = 2 * SIZE;
> > +	char		fixed[SIZE];
> > +	int		dynamically_allocated = FALSE;
> 
> That variable should have type `bool' rather than int.
> 
> > +	va_start(ap, fmt);
> > +	n = vsnprintf(fixed, SIZE, fmt, ap);
> > +	va_end(ap);
> >  
> > +	/*
> > +	** If that didn't work, use a dynamically allocated array twice
> > +	** the size of the fixed array and keep growing the array until
> > +	** the string fits.
> > +	*/
> > +	if (!(n > -1 && n < size)) {
> > +		p = MR_NEW_ARRAY(char, size);
> > +
> > +		while (1) {
> > +			dynamically_allocated = TRUE;
> > +
> > +			/* Try to print in the allocated space. */
> > +			va_start(ap, fmt);
> > +			n = vsnprintf(p, size, fmt, ap);
> > +			va_end(ap);
> > +
> > +			/* If that worked, return the string.  */
> > +			if (n > -1 && n < size) {
> > +				break;
> > +			}
> > +
> > +			/* Else try again with more space.  */
> > +			if (n > -1) {	/* glibc 2.1 */
> > +				size = n + 1; /* precisely what is needed */
> > +			} else {	/* glibc 2.0 */
> > +				size *= 2;  /* twice the old size */
> > +			}
> >  
> > +			MR_RESIZE_ARRAY(p, char, size);
> >  		}
> > +	} else {
> > +		p = fixed;
> >  	}
> 
> I think you can do that with a little less code duplication and
> somewhat simpler (IMHO) logic:
> 
> 	/*
> 	** On the first iteration, we try with a fixed-size buffer.
> 	** If that didn't work, use a dynamically allocated array twice
> 	** the size of the fixed array and keep growing the array until
> 	** the string fits.
> 	*/
> 	p = fixed;
> 
> 	while (1) {
> 		/* Try to print in the allocated space. */
> 		va_start(ap, fmt);
> 		n = vsnprintf(p, size, fmt, ap);
> 		va_end(ap);
> 
> 		/* If that worked, return the string.  */
> 		if (n > -1 && n < size) {
> 			break;
> 		}
> 
> 		/* Else try again with more space.  */
> 		if (n > -1) {	/* glibc 2.1 */
> 			size = n + 1; /* precisely what is needed */
> 		} else {	/* glibc 2.0 */
> 			size *= 2;  /* twice the old size */
> 		}
> 
> 		if (!dynamically_allocated) {
> 			p = MR_NEW_ARRAY(char, size);
> 			dynamically_allocated = TRUE;
> 		} else {
> 			MR_RESIZE_ARRAY(p, char, size);
> 		}
> 	}
> 

===================================================================


Estimated hours taken: 0.1

runtime/mercury_string.c:
	Simplify the logic of MR_make_string as suggested by Fergus
	Henderson <fjh at cs.mu.oz.au>.


Index: mercury_string.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_string.c,v
retrieving revision 1.6
diff -u -r1.6 mercury_string.c
--- mercury_string.c	2000/08/16 16:02:21	1.6
+++ mercury_string.c	2000/08/16 16:45:58
@@ -13,7 +13,7 @@
   #define vsnprintf	_vsnprintf
 #endif
 
-#define SIZE	4096
+#define BUFFER_SIZE	4096
 
 MR_String
 MR_make_string(MR_Code *proclabel, const char *fmt, ...) {
@@ -23,46 +23,42 @@
 	char		*p;
 
 #if defined(HAVE_VSNPRINTF) || defined(HAVE__VSNPRINTF)
-	int 		size = 2 * SIZE;
-	char		fixed[SIZE];
-	int		dynamically_allocated = FALSE;
+	int 		size = 2 * BUFFER_SIZE;
+	char		fixed[BUFFER_SIZE];
+	bool		dynamically_allocated = FALSE;
 	
-	va_start(ap, fmt);
-	n = vsnprintf(fixed, SIZE, fmt, ap);
-	va_end(ap);
-
 	/*
+	** On the first iteration we try with a fixed-size buffer.
 	** If that didn't work, use a dynamically allocated array twice
 	** the size of the fixed array and keep growing the array until
 	** the string fits.
 	*/
-	if (!(n > -1 && n < size)) {
-		p = MR_NEW_ARRAY(char, size);
+	p = fixed;
 
-		while (1) {
-			dynamically_allocated = TRUE;
+	while (1) {
+		/* Try to print in the allocated space. */
+		va_start(ap, fmt);
+		n = vsnprintf(p, size, fmt, ap);
+		va_end(ap);
+
+		/* If that worked, return the string.  */
+		if (n > -1 && n < size) {
+			break;
+		}
 
-			/* Try to print in the allocated space. */
-			va_start(ap, fmt);
-			n = vsnprintf(p, size, fmt, ap);
-			va_end(ap);
-
-			/* If that worked, return the string.  */
-			if (n > -1 && n < size) {
-				break;
-			}
-
-			/* Else try again with more space.  */
-			if (n > -1) {	/* glibc 2.1 */
-				size = n + 1; /* precisely what is needed */
-			} else {	/* glibc 2.0 */
-				size *= 2;  /* twice the old size */
-			}
+		/* Else try again with more space.  */
+		if (n > -1) {   /* glibc 2.1 */
+			size = n + 1; /* precisely what is needed */
+		} else {        /* glibc 2.0 */
+			size *= 2;  /* twice the old size */
+		}
 
+		if (!dynamically_allocated) {
+			p = MR_NEW_ARRAY(char, size);
+			dynamically_allocated = TRUE;
+		} else {
 			MR_RESIZE_ARRAY(p, char, size);
 		}
-	} else {
-		p = fixed;
 	}
 
 #else

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------