save breakpoints does not save signal catchpoints correctly

  Hi,

Miroslav Franc submitted a fix
	https://bugzilla.redhat.com/show_bug.cgi?id=1146170#c0

so posting it with a testcase.

No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu.


Jan
gdb/
2014-10-03  Miroslav Franc  <mfranc@redhat.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix "save breakpoints" for "catch" command.
	* break-catch-sig.c (signal_catchpoint_print_recreate): Add trailing
	newline.

gdb/testsuite/
2014-10-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix "save breakpoints" for "catch" command.
	* gdb.base/catch-signal.exp: Add gdb_breakpoint "main".
	Remove -nonewline.  Match also the added "main" line.
  

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

The fix looks right to me.  Just one comment on the test case...

> -    set contents [read -nonewline $fd]
> +    set contents [read $fd]
>      close $fd
>  

IWBN to split the contents into a list and match each element in the
list one by one, so that ...

> +    set nl "\r?\n"
>      if {$arg == ""} {
> -	set pattern "catch signal"
> +	set pattern "catch signal$nl"
>      } else {
> -	set pattern "catch signal $arg"
> +	set pattern "catch signal $arg$nl"
>      }
> -    if {[string match $pattern $contents]} {
> +    set pattern "${pattern}break main$nl"
> +    if {[regexp "$pattern" $contents]} {

... the pattern can be simplified and we can still use "string match".

On the other hand, in this way, the test can be easily extended in the
future, for example, saving more breakpoints in the file.
  

On Thu, 09 Oct 2014 15:37:24 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> The fix looks right to me.  Just one comment on the test case...
> 
> > -    set contents [read -nonewline $fd]
> > +    set contents [read $fd]
> >      close $fd
> >  
> 
> IWBN to split the contents into a list and match each element in the
> list one by one, so that ...
> 
> > +    set nl "\r?\n"
> >      if {$arg == ""} {
> > -	set pattern "catch signal"
> > +	set pattern "catch signal$nl"
> >      } else {
> > -	set pattern "catch signal $arg"
> > +	set pattern "catch signal $arg$nl"
> >      }
> > -    if {[string match $pattern $contents]} {
> > +    set pattern "${pattern}break main$nl"
> > +    if {[regexp "$pattern" $contents]} {
> 
> ... the pattern can be simplified and we can still use "string match".
> 
> On the other hand, in this way, the test can be easily extended in the
> future, for example, saving more breakpoints in the file.

Sorry but be more specific how you wanted to simplify the code.
Any idea I have is 10x more complicated.

With the list I have Googled
	What is the right way of comparing two lists in TCL?
	https://stackoverflow.com/questions/5195153/what-is-the-right-way-of-comparing-two-lists-in-tcl

and those solutions seem too complicated to me.

gdb_expect_list or gdb_test_sequence are not usable here as this is not an
output from inferior but just contents of a file.


Thanks,
Jan
  

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> Sorry but be more specific how you wanted to simplify the code.
> Any idea I have is 10x more complicated.
>
> With the list I have Googled
> 	What is the right way of comparing two lists in TCL?
> 	https://stackoverflow.com/questions/5195153/what-is-the-right-way-of-comparing-two-lists-in-tcl
>
> and those solutions seem too complicated to me.

I didn't mean to compare two lists.  We only have one list here, and we
can check each element in it, for example,

     set fd [open $filename]
-    set contents [read -nonewline $fd]
+    set file_data [read $fd]
+    set data [split $file_data "\n"]
+    set contents [lindex $data 0]
+
     close $fd

contents is the first line of the file, and the pattern matching code
can be unchanged.  After that, we can check the second line of the file,
which should be "break main".  The test becomes,

    set fd [open $filename]
    set file_data [read $fd]
    set data [split $file_data "\n"]
    close $fd

    if {$arg == ""} {
	set pattern "catch signal"
    } else {
	set pattern "catch signal $arg"
    }
    # Check the first line.
    gdb_assert {[string match $pattern [lindex $data 0]]} \
	"1st line of save breakpoints for '$arg'"
    # Check the second line.
    gdb_assert {[string match "break main" [lindex $data 1]]} \
	"2nd line of save breakpoints for '$arg'"

This will be more clear and easier to extend when there are more lines in
the file.