[m-dev.] diff: MR_make_string improvement

[Fergus Henderson]

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);
		}
	}

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------