[m-dev.] for review: string__fmt

[Fergus Henderson]

On 10-Aug-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> > > +:- type spec
> > > +		% valid integer specifiers
> > > +	--->	d(int)
> > > +	;	i(int)
> > > +	;	o(int)
> > > +	;	u(int)
> > > +	;	x(int)
> > > +	;	cX(int)
> > 
> > The meaning of "cX" was a mystery to me for a while.
> > At very least there should be a comment explaining it.
> > But I think it would make more sense to use "'X'"
> > rather than "cX".
>
> I tried that but in some spots it was still being parsed as a variable.

Hmm, I don't believe you ;-)

> I will try an reconstruct it as a test case.  For the moment a comment
> has been added.

OK.

> > Please use curly braces for the bodies of `if' and `else'
> > statements.  Please put spaces around operators such as `+'.
> > 
> 
> Here is the revised code

> MR_String
> MR_make_string(MR_Code *proclabel, const char *fmt, ...) {

That `{' should be on a line of its own.

> 		/* Else try again with more space.  */
> 		if (n > -1)    /* glibc 2.1 */
> 			 /* precisely what is needed */
> 			 size = n + 1;
> 		else           /* glibc 2.0 */
> 			size *= 2;  /* twice the old size */

Please use curly braces for the bodies of `if' and `else' statements!

Also fix the indentation -- the "then" part is intended one space
too many.

> 		/* It is possible for this buffer to overflow	*/
> 		/* and then bad things may happen		*/
> 	char p[40960];

Please use

	/*
	** ...
	*/

format for comments.

> 	strcpy((char *) result, p);

Why the cast to `char *' here?  MR_String is already `char *', isn't it?
And if it wasn't, e.g. if it was `wchar_t *', then surely we'd want a
compiler diagnostic here, but the cast would supress that.

Apart from that, this looks fine now.

-- 
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
--------------------------------------------------------------------------