[PATCH] mi-stack.exp

[PATCH] mi-stack.exp

[Nick Roberts]

2005-06-20  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.mi/mi-stack.exp (test_stack_locals_listing): 
	Remove test for -stack-select-frame without arguments.
	(test_stack_frame_listing): 
	Add test for newly implemented command -stack-info-frame.


*** /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp.~1.15.~	2005-05-18 20:18:15.000000000 +1200
--- /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp	2005-06-19 18:49:18.000000000 +1200
***************
*** 55,60 ****
--- 55,61 ----
      # -stack-list-frames
      # -stack-list-frames 1 1
      # -stack-list-frames 1 3
+     # -stack-info-frame
  
      mi_gdb_test "231-stack-list-frames" \
  	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
***************
*** 69,74 ****
--- 70,79 ----
      mi_gdb_test "234-stack-list-frames 1" \
  	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
  	    "stack frame listing wrong"
+ 
+     mi_gdb_test "235-stack-info-frame" \
+             "235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\}" \
+ 	       "selected frame listing"
  }
  
  proc test_stack_args_listing {} {
***************
*** 187,202 ****
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals listing for new frame"
  
- # this should be a no-op
- 
-     mi_gdb_test "232-stack-select-frame" \
- 	    "232\\^done" \
-                 "stack select same frame"
- 
      mi_gdb_test "232-stack-list-locals 1" \
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals for same frame (level 1)"
- 
  }
  
  mi_runto callee4
--- 192,200 ----

Re: [PATCH] mi-stack.exp

[Daniel Jacobowitz]

On Mon, Jun 20, 2005 at 12:57:34PM +1200, Nick Roberts wrote:
> 2005-06-20  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* gdb.mi/mi-stack.exp (test_stack_locals_listing): 
> 	Remove test for -stack-select-frame without arguments.
> 	(test_stack_frame_listing): 
> 	Add test for newly implemented command -stack-info-frame.

OK.  Please wrap it this way:

	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
	-stack-select-frame without arguments.
	(test_stack_frame_listing): Add test for newly implemented command
	-stack-info-frame.

> *** /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp.~1.15.~	2005-05-18 20:18:15.000000000 +1200
> --- /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp	2005-06-19 18:49:18.000000000 +1200

Could you see about submitting relative diffs?  It doesn't really
matter what they're relative _to_ - common is -p0 or -p1 to the top
directory whose ChangeLog you're touching.  But this is a bit awkward
to apply.

As an aside, if you don't mind, please check in patches all at once. 
When you check in each file separately with its own log message,
automated tools can't reconstruct the entire change.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] mi-stack.exp

[Nick Roberts]

 > > 	* gdb.mi/mi-stack.exp (test_stack_locals_listing): 
 > > 	Remove test for -stack-select-frame without arguments.
 > > 	(test_stack_frame_listing): 
 > > 	Add test for newly implemented command -stack-info-frame.
 > 
 > OK.  Please wrap it this way:
 > 
 > 	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
 > 	-stack-select-frame without arguments.
 > 	(test_stack_frame_listing): Add test for newly implemented command
 > 	-stack-info-frame.

OK. (I find the log message easier to read when it fits on one line)

 > > *** /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp.~1.15.~ 2005-05-18
 > > 20:18:15.000000000 +1200 ---
 > > /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp 2005-06-19
 > > 18:49:18.000000000 +1200
 > 
 > Could you see about submitting relative diffs?  It doesn't really
 > matter what they're relative _to_ - common is -p0 or -p1 to the top
 > directory whose ChangeLog you're touching.  But this is a bit awkward
 > to apply.

Like below? (This is with vc-diff in Emacs)

 > As an aside, if you don't mind, please check in patches all at once. 
 > When you check in each file separately with its own log message,
 > automated tools can't reconstruct the entire change.

You mean, for example, the log message:

	* mi/mi-cmd-stack.c (mi_cmd_stack_info_frame): New function.

	* mi/mi-cmds.c (mi_cmds): Replace entry for -stack-info-frame.
	Make it use mi_cmd_stack_info_frame

	* mi/mi-cmds.h (mi_cmd_stack_info_frame): New declaration.

should appear in the individual CVS log for each of the files
mi/mi-cmd-stack.c, mi/mi-cmds.c and mi/mi-cmds.h?  I thought that the
ChangeLog was there for that purpose.

Nick


2005-06-20  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
        -stack-select-frame without arguments.
	(test_stack_frame_listing): Add test for newly implemented command
        -stack-info-frame.


*** mi-stack.exp	18 May 2005 20:18:15 +1200	1.15
--- mi-stack.exp	19 Jun 2005 18:49:18 +1200	
***************
*** 55,60 ****
--- 55,61 ----
      # -stack-list-frames
      # -stack-list-frames 1 1
      # -stack-list-frames 1 3
+     # -stack-info-frame
  
      mi_gdb_test "231-stack-list-frames" \
  	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
***************
*** 69,74 ****
--- 70,79 ----
      mi_gdb_test "234-stack-list-frames 1" \
  	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
  	    "stack frame listing wrong"
+ 
+     mi_gdb_test "235-stack-info-frame" \
+             "235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\}" \
+ 	       "selected frame listing"
  }
  
  proc test_stack_args_listing {} {
***************
*** 187,202 ****
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals listing for new frame"
  
- # this should be a no-op
- 
-     mi_gdb_test "232-stack-select-frame" \
- 	    "232\\^done" \
-                 "stack select same frame"
- 
      mi_gdb_test "232-stack-list-locals 1" \
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals for same frame (level 1)"
- 
  }
  
  mi_runto callee4

Re: [PATCH] mi-stack.exp

[Daniel Jacobowitz]

On Mon, Jun 20, 2005 at 02:27:39PM +1200, Nick Roberts wrote:
>  > OK.  Please wrap it this way:
>  > 
>  > 	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
>  > 	-stack-select-frame without arguments.
>  > 	(test_stack_frame_listing): Add test for newly implemented command
>  > 	-stack-info-frame.
> 
> OK. (I find the log message easier to read when it fits on one line)

Yeah, it is kinda - but this is the way all the others look, more
consistently.

>  > > *** /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp.~1.15.~ 2005-05-18
>  > > 20:18:15.000000000 +1200 ---
>  > > /home/nick/src/gdb/testsuite/gdb.mi/mi-stack.exp 2005-06-19
>  > > 18:49:18.000000000 +1200
>  > 
>  > Could you see about submitting relative diffs?  It doesn't really
>  > matter what they're relative _to_ - common is -p0 or -p1 to the top
>  > directory whose ChangeLog you're touching.  But this is a bit awkward
>  > to apply.
> 
> Like below? (This is with vc-diff in Emacs)

Ideally, something like "testsuite/gdb.mi/mi-stack.exp"; the result of
"cvs diff" from within the gdb/ directory.  Or for things outside of gdb,
within the top level directory.

The goal is to provide something that can easily be applied.  To apply
the above, I need to work out that /home/nick/src/gdb is -p4.  But if
it just said testsuite/, I'd know immediately that it was -p0 from the
gdb directory.  With just "mi-stack.exp" we have the opposite problem;
imagine if you were patching Makefile.

> You mean, for example, the log message:
> 
> 	* mi/mi-cmd-stack.c (mi_cmd_stack_info_frame): New function.
> 
> 	* mi/mi-cmds.c (mi_cmds): Replace entry for -stack-info-frame.
> 	Make it use mi_cmd_stack_info_frame
> 
> 	* mi/mi-cmds.h (mi_cmd_stack_info_frame): New declaration.
> 
> should appear in the individual CVS log for each of the files
> mi/mi-cmd-stack.c, mi/mi-cmds.c and mi/mi-cmds.h?  I thought that the
> ChangeLog was there for that purpose.

The idea is to use a single "cvs commit" to check in all the files that
you've modified.  So, yes, one log message for them all.  Take a look
at "cvs log" output on a couple files, or the archives at
http://sourceware.org/ml/gdb-cvs/.

> 
> 2005-06-20  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
>         -stack-select-frame without arguments.
> 	(test_stack_frame_listing): Add test for newly implemented command
>         -stack-info-frame.

OK (but be careful of leading tabs in the changelog).

-- 
Daniel Jacobowitz
CodeSourcery, LLC