[m-rev.] for review: mslice and mdice

[Ian MacLarty]

On Fri, Apr 01, 2005 at 05:25:10PM +1000, Zoltan Somogyi wrote:
> For review by Ian. The documentation is still to come.

Please also supply some test cases for the `slice' command and the new sorting
options.

> 
> trace/mercury_trace_internal.c:
> 	Generalize the code for specifying dices to allow either or both
> 	of the slices being subtracted to be specified as the union of
> 	one or more trace counts files.
> 
> 	Fix several places where we weren't checking the return value of
> 	malloc. Fix two places where we could conceivably free strings that
> 	were still alive. Fix some places where we could pass NULL strings
> 	to Mercury code, and some places where we could free NULL pointers
> 	(which is not guaranteed to work on all platforms).
> 
> 	Use existing functions such as MR_copy_string where appropriate.
> 
> 	Add a new command, slice, for printing information on slices.

Will this really be useful from within mdb?

> 
> tests/run_one_test:
> 	Fix two small bugs in this script: make the filenames more user
> 	friendly, and make sure that gzip isn't asked to overwrite an
> 	existing file, since that causes it to ask a question on stdout
> 	and to wait for an answer.
> 
> Index: browser/dice.m
> ===================================================================

Should dice.m maybe just be removed altogether?

> Index: library/list.m
> ===================================================================

This file isn't mentioned in the CVS log and map7 and map8 don't seem
to be used in this diff.

> Index: mdbcomp/trace_counts.m
> ===================================================================
> +    io::di, io::uo) is det.
> +
> +:- type slice_source
> +    --->    file_list
> +    ;       single_file
> +    ;       try_single_first.
> +
> +:- type read_trace_counts_source_result
> +    --->    source_ok(set(trace_count_file_type), list(trace_counts))

Shouldn't this be a list instead of a set?  Surely the caller might want to 
relate each file type to the trace_counts the file generated?

> +    ;       source_error_message(string).
> +
> +    % read_trace_counts_source(Source, FileName, Result, !IO):
> +    %
> +    % Read in trace counts stored in one or more trace count files.
> +    %
> +    % If Source is file_list, then FileName should contain a list of filenames,
> +    % and read_trace_counts_source will read in the trace counts from each
> +    % of those files.
> +    %
> +    % If Source is single_file, then FileName should itself be a file
> +    % containing trace counts data.
> +    %
> +    % If Source is try_single_first, then read_trace_counts_source will
> +    % check to see if FileName contains trace counts data. If yes, it will
> +    % get trace counts from there. If not, read_trace_counts_source will
> +    % interpret FileName as a file that itself contains filenames, and
> +    % will treat it as with Source=file_list.
> +    %
> +    % The list of file types returned will contain the file types of all

s/list/set/ if it should be a set.

...
>  
>  read_trace_counts(FileName, ReadResult, !IO) :-
> -	io__open_input(FileName, Result, !IO),
> +    % XXX We should be using zcat here, to avoid deleted the gzipped file

s/deleted/deleting

> Index: runtime/mercury_trace_base.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_trace_base.h,v
> retrieving revision 1.46
> diff -u -b -r1.46 mercury_trace_base.h
> --- runtime/mercury_trace_base.h      31 Mar 2005 04:44:30 -0000      1.46
> +++ runtime/mercury_trace_base.h      31 Mar 2005 07:54:02 -0000
> @@ -108,8 +108,14 @@
>  **
>  ** The dummy argument allows this function to be registered with
>  ** MR_register_exception_cleanup.
> +**
> +** The file can be recognized as a Mercury trace counts file as its first
> +** line matches MR_TRACE_COUNT_FILE_ID. The value of that macro should be
> +** kept in sync with
>  */
>

Finish this comment.

Otherwise that looks fine.

Cheers,
Ian.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------