debug function errors

John W. Eaton jwe at bevo.che.wisc.edu
Fri Jan 4 15:49:34 CST 2008


On 28-Dec-2007, John Swensen wrote:

| In my recent patches to the debugging functions to make them more like  
| Matlab, I discovered a few bugs.  In particular, a call to dbclear on  
| a file with no breakpoints or without any parameter (should clear all  
| breakpoints) caused Octave to crash.  Also, a call to dbstop with no  
| parameters causes a crash, where it should just report and error.
| 
| I have attached a patch for the problem.  I included this on an email  
| to the maintainers list earlier, but I think my patch file was  
| malformed because I tried to apply it to the latest from CVS and diff  
| complained.  Attached is a new patch I just generated from 'cvs diff'  
| after applying the old patch by hand to the latest CVS code.
| 
| Changlog entry
| ----------------------
| 2007-12-27  John Swensen  <jpswensen at comcast.net>
| 
|          * debug.cc: Fixed bug dbtsop and dbclear introduced when  
| making them more compatible

ChangeLog entries should list function names and state what was
changed.  Explanations about why the changes were made aren't needed,
but if the change is tricky, it helps to add some explanation as a
comment in the code.  For example,

2008-01-04  John Swensen  <jpswensen at comcast.net>

	* debug.cc (bp_table::do_remove_all_breakpoints_in_file):
	Avoid calling erase on invalid bp_map iterators.
	(bp_table::do_remove_breakpoint): Only try to delete breakpoints
	if some exist.  Avoid calling erase on invalid bp_map iterators.
	(parse_dbfunction_params): Return early if ARGS is empty.
	New arg, WHO.  Change all uses.
	Accept but do nothing with struct args.

| +     error ("Illegal parameter specified");

Please avoid the use of "illegal" in messages (no laws are broken
when the function is called incorrectly).  It is better so use
"invalid".  Also, it's best to include the name of the function in an
error message, so I changed this to

  error ("%s: invalid parameter specified", who);

and added WHO as an arguement to the parse_... function.

| + 	  octave_stdout << "Accepting a struct" << std::endl;

Likewise, I changed this to

  octave_stdout << who << ": accepting a struct" << std::endl;


I checked in the changes on cvs head and the 3.0 branch.

Thanks,

jwe


More information about the Bug-octave mailing list