This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Make the prec support signal better[0/4]


I cannot really claim that I'm an expert, but this definitely looks
a lot better.

The C file needs a copyright header as well. I also noticed that your
C file handles K&R syntax (was it actually K&R where the type of the
function arguments is provided separately after the function name and
parameters are specified?), which I think is superfluous. But I think
you inherited it, and I wouldn't worry about it.

> 	gdb_test_multiple "continue" "get signal" {
> 	    -re "Program received signal SIG$thissig.*handle_$thissig.*$gdb_prompt $" {
> 		fail "get signal $thissig (wrong location)"
> 	    }
> 	    -re "Program received signal SIG$thissig.*$gdb_prompt $" {
> 		pass "get signal $thissig"
> 	    }
> 	    -re "Breakpoint.* handle_$thissig.*$gdb_prompt $" {
> 		xfail "get signal $thissig"
> 		set need_another_continue 0
> 	    }
> 	    -re ".*$gdb_prompt $" {
> 		fail "get signal $thissig"
> 		set need_another_continue 0
> 	    }
> 	    default {
> 		fail "get signal $thissig (eof or timeout)"
> 	    }

As far as I know, you do not need the "default" section.
This is already handled by gdb_test_multiple. There is at least
one more default in your testcase that you can probably remove.

In terms of style, most tests I've seen are written with the name of
the test stored in a variable, so that they don't have to copy that name
around inside very case of the test_multiple:

    set test "$prefix; leave handler"
    gdb_test_multiple "$i" "${test}" {
        -re "Could not insert single-step breakpoint.*$gdb_prompt $" {
            fail "$test (could not insert single-step breakpoint)"
        }
        -re "Program exited normally.*${gdb_prompt} $" {
            fail "$test (program exited)"
        }
        -re "(while ..done|done = 0).*${gdb_prompt} $" {
            pass "$test"
        }

> # gdb_continue_to_end "continue to sigall exit"

I am guessing that you did not mean to leave this commented-out code
:-)

> gdb_test_multiple "echo foo\n" "test echo foo" {
>     -re ".*foo.*$gdb_prompt " {
> 	pass "echo foo"
>     }
>     -re ".*$gdb_prompt " {
> 	fail "echo foo"
>     }

Great test! ;-) At first, I wasn't really paying attention, and I was
going to say that you don't need gdb_test_multiple, since gdb_test
would have been perfectly sufficient. And then, Ooooohhhhw...

> return 0

I think this one can go as well.

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]