[m-rev.] Updated diff for deep profiling.

Thomas Conway conway at cs.mu.OZ.AU
Sat May 19 10:02:15 AEST 2001


On Fri, May 18, 2001 at 10:24:25PM EST, Fergus Henderson wrote:
> > +static void
> > +MR_write_num(FILE *fp, unsigned long num)
> > +{
> > +	unsigned char	pieces[sizeof(unsigned long) * 8 / 7 + 1];
> > +	int		i;
> > +
> > +#ifdef	MR_DEEP_PROFILING_DETAIL_DEBUG
> > +	fprintf(debug_fp, "num: %ld\n", num);
> > +#endif
> > +
> > +	MR_deep_assert((int) num >= 0);
> > +
> > +	i = 0;
> > +	do {
> > +		pieces[i] = num & 0x7f;
> > +		num = num >> 7;
> > +		i++;
> > +	} while (num != 0);
> > +
> > +	i--;
> > +
> > +	while (i > 0) {
> > +		putc(pieces[i--] | 0x80, fp);
> > +	}
> > +	putc(pieces[0], fp);
> > +}
> 
> A comment here explaining the encoding would be helpful.
> 
> There's a lot of magic numbers in this code.
> The code would be easier to understand if these were named.

Actually, I think the magic numbers here are clear, but an
additional comment like

	/*
	** Write out a (non-negative) integer.
	** The format we use is a multibyte format
	** which uses the least significant 7 bits
	** as data bits and the most significant bit
	** to indicate whether there are more bytes
	** following. Numbers are written most
	** significant byte first.
	*/

In this case, using symbolic names for the bit masks would
obscure the meaning of the code more than they helped.

-- 
  Thomas Conway )O+
 <conway at cs.mu.oz.au>       499 User error! Replace user, and press any key.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list