Re: -stack-list-frames HIGH_FRAME changes

[Vladimir Prus <ghost@cs.msu.su>]

[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]

On Friday 05 May 2006 17:51, Daniel Jacobowitz wrote:
> On Fri, May 05, 2006 at 11:54:08AM +0400, Vladimir Prus wrote:
> > Hi,
> > attached patch stops -stack-list-frames from emitting error when the
> > HIGH_FRAME parameter is larger than the actual number of frames.
>
> As far as I'm concerned, this change is reasonable; but let's wait
> until next week to see if anyone else has a reason for the current
> behavior. (I don't think anyone will; just being cautious.)
>
> > ? .gdbinit
>
> Please trim all this next time :-)

Sure, sorry.

> > ===================================================================
> > RCS file: /cvs/src/src/gdb/ChangeLog,v
> > retrieving revision 1.7714
> > diff -u -r1.7714 ChangeLog
> > --- ChangeLog	3 May 2006 22:59:38 -0000	1.7714
> > +++ ChangeLog	5 May 2006 07:50:28 -0000
> > @@ -1,3 +1,10 @@
>
> It's usually better to include ChangeLog entries as text, not as diffs.
> When they're diffs, they invariably generate patch rejects later on.

The reason I've included them in patch is that I've learned about Emacs's 
add-change-log-entry command, which conveniently adds entry to existing 
file ;-) So, in case if there are several ChangeLog files in patch, I should 
put those entries separately, and indicate which file each entry goes to, 
right?

> > +2006-05-05  Vladimir Prus  <ghost@lvk.cs.msu.su>
> > +
> > +	* mi/mi-cmd-stack.c (mi_cmd_stack_list_frames): Don't emit error
> > +	if high requested frame number is larger then number of available
> > +	frames.
> > +
> > +
>
> Just one blank line between entries, please.

Ok.

> > -are equal, it shows the single frame at the corresponding level.
> > +are equal, it shows the single frame at the corresponding level. It is
> > +an error if @var{low-frame} is larger than the actual number of frames.
> > On +the other hand, @var{high-frame} may be larger then the actual number
> > of +frames, in which case only existing frames will be returned.
>
> Two spaces after periods.  

Ok.

>
> > Index: testsuite/ChangeLog
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
> > retrieving revision 1.1208
> > diff -u -r1.1208 ChangeLog
> > --- testsuite/ChangeLog	1 May 2006 22:21:35 -0000	1.1208
> > +++ testsuite/ChangeLog	5 May 2006 07:50:34 -0000
> > @@ -1,3 +1,9 @@
> > +2006-05-05  Vladimir Prus  <ghost@lvk.cs.msu.su>
> > +
> > +	* gdb.mi/mi2-stack.exp (test_stack_frame_listing): Test that
> > +	HIGH_FRAME argument to -stack-list-locals can be larger than
> > +	the number of frames.
>
> Please don't add new tests to just mi2-stack.exp.  I don't much care
> whether mi2-stack.exp is updated or not, but when we finalize mi3 and
> create mi3-* tests, they're going to be copied from mi-*.  So it's
> important that new tests go there also.

So, what's the point of mi2 tests at all? 
In any case, there's revised patch:

./Changelog:
2006-05-05  Vladimir Prus  <ghost@lvk.cs.msu.su>

	* mi/mi-cmd-stack.c (mi_cmd_stack_list_frames): Don't emit error
	if high requested frame number is larger then number of available 
	frames.


doc/ChangeLog
2006-05-05  Vladimir Prus  <ghost@lvk.cs.msu.su>

	* gdb.texinfo (GDB/MI Stack Manipulation): Mention that
	-stack-list-locals HIGH_FRAME argument can be larger then the
	actual number of frames.


testsuite/ChangeLog

2006-05-05  Vladimir Prus  <ghost@lvk.cs.msu.su>

	* gdb.mi/mi-stack.exp (test_stack_frame_listing): Test that
	HIGH_FRAME argument to -stack-list-locals can be larger than
	the number of frames.


- Volodya


[-- Attachment #2: %40-stack-list-frames.diff --]
[-- Type: text/x-diff, Size: 2354 bytes --]

Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.325
diff -u -r1.325 gdb.texinfo
--- doc/gdb.texinfo	27 Apr 2006 23:03:42 -0000	1.325
+++ doc/gdb.texinfo	5 May 2006 14:37:09 -0000
@@ -19922,7 +19922,10 @@
 If invoked without arguments, this command prints a backtrace for the
 whole stack.  If given two integer arguments, it shows the frames whose
 levels are between the two arguments (inclusive).  If the two arguments
-are equal, it shows the single frame at the corresponding level.
+are equal, it shows the single frame at the corresponding level.  It is
+an error if @var{low-frame} is larger than the actual number of
+frames.  On the other hand, @var{high-frame} may be larger then the
+actual number of frames, in which case only existing frames will be returned.
 
 @subsubheading @value{GDBN} Command
 
Index: mi/mi-cmd-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
retrieving revision 1.29
diff -u -r1.29 mi-cmd-stack.c
--- mi/mi-cmd-stack.c	23 Dec 2005 18:57:46 -0000	1.29
+++ mi/mi-cmd-stack.c	5 May 2006 14:37:09 -0000
@@ -88,8 +88,6 @@
     }
 
   do_cleanups (cleanup_stack);
-  if (i < frame_high)
-    error (_("mi_cmd_stack_list_frames: Not enough frames in stack."));
 
   return MI_CMD_DONE;
 }
Index: testsuite/gdb.mi/mi-stack.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-stack.exp,v
retrieving revision 1.19
diff -u -r1.19 mi-stack.exp
--- testsuite/gdb.mi/mi-stack.exp	27 Sep 2005 22:39:02 -0000	1.19
+++ testsuite/gdb.mi/mi-stack.exp	5 May 2006 14:37:10 -0000
@@ -74,6 +74,10 @@
     mi_gdb_test "235-stack-info-frame" \
             "235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\}" \
 	       "selected frame listing"
+
+    mi_gdb_test "236-stack-list-frames 1 300" \
+	    "236\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\}\\\]" \
+                "stack frame listing 1 300"
 }
 
 proc test_stack_args_listing {} {