[m-rev.] Re: for review: user-friendly representation of streams

[Fergus Henderson]

> The medium change is that the predicates to report stats now return the
> statistics as strings, instead of printing them out. Amongst other things,
> this allows the stats to be printed to streams other than the current one.
> 
> library/io.m:
> 	Add predicates to report statistics to user-supplied streams.

- The changes to library/benchmarking.m are not mentioned
  in the log message.
- if you're changing user-visible stuff, you should mention
  it in the NEWS file
- The change breaks backwards compatibility, I think.
  This is in general undesirable and in this case unnecessary.
  It would be easy enough to keep the old interface
  (perhaps adding `pragma obsolete', if this is desired)
  as a wrapper around the new functionality.

> Index: library/benchmarking.m
>  :- module benchmarking.
>  :- interface.
>  
> -% `report_stats' is a non-logical procedure intended for use in profiling
> +% `report_stats' is a non-logical function intended for use in profiling

You've changed the comment here to say it is a function, but in the code
it remains a predicate.

> -void
> +MR_String
>  ML_report_stats(void)

The lifetime of the return value should be documented here.
If the caller is responsible for deallocating the memory,
then this should be documented.

> -:- pragma foreign_proc("C", report_stats,
> +:- pragma foreign_proc("C",
> +	report_stats(Stats::out),
>  	[will_not_call_mercury],
>  "
> -	ML_report_stats();
> +	MR_String	Stats0;
> +
> +	Stats0 = ML_report_stats();
> +	MR_make_aligned_string_copy(Stats, Stats0);
>  ").

Won't this be a memory leak, due to the memory allocated for the return
value of ML_report_stats() not being deallocated?

Likewise for report_full_memory_stats.

> +	profile_buf = MR_malloc(profile_buf_size);
> +	if (profile_buf == NULL) {
> +		MR_fatal_error(""ML_report_stats: out of memory"");
> +	}

This check (and others like it) are not needed; MR_malloc() guarantees
that the return value will not be null.

> +	snprintf(profile_buf, profile_buf_size,

As mentioned in my other mail, snprintf() is not portable.

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