This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] A patch for parse_number (c-exp.y) to recognize 1.25f
Hi Daniel,
> This looks good. I have some comments on formatting and testing, but
> that's it.
Thanks for your comment. They are very helpful and informative indeed.
> >
> > * c-exp.y (parse-number): Modify the float parsing logic to let it
> > recognize the suffix.
>
> "a suffix" rather than "the suffix", please.
Changed.
>
> > ! if (num == 1)
> > ! putithere->typed_val_float.type = builtin_type (current_gdbarch)
> > ! ->builtin_double;
>
> Instead of splitting this line (and the two below) in the middle of a
> single reference, you can split it at the assignment. Like this - two
> spaces in the continued line:
>
> putithere->typed_val_float.type
> = builtin_type (current_gdbarch)->builtin_double;
>
> Emacs'll butcher your version, I'm afraid.
I am not familar with Emacs. But I believe that you are right. Fixed.
[snip]
>
> It's a pretty trivial test case; but please add a copyright notice
> anyway.
Added.
>
> > + # Run to the breakpoint at return.
> > + set bp_location [gdb_get_line_number "return"]
> > + gdb_test "break $bp_location" \
> > + "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> > + "breakpoint at return"
> > + gdb_test "continue" \
> > + "Continuing\\..*Breakpoint.*" \
> > + "continue to breakpoint"
>
> This is fine, but you can also use gdb_breakpoint and
> gdb_continue_to_brekpoint or gdb_continue.
Switched to gdb_breakpoint and gdb_continue_to_brekpoint.
>
> > + # Test that gdb could handle the above correctly with "set var" command.
> > + send_gdb "set var b32=10.5f\n"
> > + gdb_expect {
> > + -re "$gdb_prompt $" { pass "set var b32=10.5f" }
> > + -re "Invalid number" { fail "do not recognize 10.5f" }
> > + timeout {fail "set var b32=10.5f" }
> > + }
>
> Please don't use "send_gdb" and "gdb_expect" directly. Also, the
> passing and failing tests should have the same message - an optional
> string at the end in parentheses explaining the failure is OK, but
> anything else is bad for automated testing.
Good guideline. I am now using gdb_test_multiple.
I had run the modified testcase against patched and un-patched GDB. It
report expected test result. Both the patch and testcases are checked in.
Thanks.
Regards
- Wu Zhou