[m-rev.] for review: mdb ignore counts

[Fergus Henderson]

On 03-Aug-2001, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Associate optional ignore counts with breakpoints.
...
> +++ doc/user_guide.texi	2001/08/02 03:07:44
> @@ -2390,7 +2390,7 @@
>  @cindex Breakpoints
>  @sp 1
>  @table @code
> - at item break [-PS] @var{filename}:@var{linenumber}
> + at item break [-PS] [-E at var{IC}] [-I at var{IC}] @var{filename}:@var{linenumber}

`-E at var{IC}' will come out as just `-EIC' in the info files,
which would be confusing, IMHO.

I suggest you change it to `-E at var{ignore-count}', or perhaps just
`-E at var{count}' and likewise for `-I' and throughout the rest of this file.

> +The option @samp{-E at var{IC}} tells the debugger
> +to ignore the breakpoint until after @var{IC} occurrences of a call event
> +that matches the breakpoint.
> +The option @samp{-I at var{IC}} tells the debugger
> +to ignore the breakpoint until after @var{IC} occurrences of interface events
> +that match the breakpoint.

The long option equivalents for `-E' and `-I' (`--ignore-entry' and
`--ignore-interface') should be documented here.

> Index: trace/mercury_trace_internal.c
...
> +		/* The value of ignore_when doesn't matter */
> +		/* while ignore_count contains zero. */

The comment layout doesn't match our coding guidelines.

> @@ -1618,6 +1725,41 @@
> +	} else if (streq(words[0], "scope")) {

The "scope" mdb command should be documented in doc/user_guide.texi,
or there should be some comments explaining why it isn't documented.

> Index: trace/mercury_trace_spy.c
> @@ -360,6 +405,11 @@
>  
>  	if (new_size == old_size) {
>  		/* there were no matching labels */
> +		char	buf[1024];	/* this should be big enough ... */
> +
> +		snprintf(buf, 1024, "there is no event at %s:%d",
> +			filename, linenumber);
> +		*problem = buf;
>  		return -1;

Two problems:
	- snprintf() is not portable
	- you are returning the address of a local variable,
	  which will get deallocated when you return


Otherwise that change looks fine.  Please post a relative diff
when you've addressed those review comments.

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