From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22454 invoked by alias); 20 Sep 2005 09:18:33 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 22425 invoked by uid 22791); 20 Sep 2005 09:18:22 -0000 Received: from ausmtp02.au.ibm.com (HELO ausmtp02.au.ibm.com) (202.81.18.187) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Tue, 20 Sep 2005 09:18:22 +0000 Received: from sd0208e0.au.ibm.com (d23rh904.au.ibm.com [202.81.18.202]) by ausmtp02.au.ibm.com (8.12.10/8.12.10) with ESMTP id j8K9CqmC238468 for ; Tue, 20 Sep 2005 19:12:52 +1000 Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.250.243]) by sd0208e0.au.ibm.com (8.12.10/NCO/VERS6.7) with ESMTP id j8K9KuWM140376 for ; Tue, 20 Sep 2005 19:20:58 +1000 Received: from d23av02.au.ibm.com (loopback [127.0.0.1]) by d23av02.au.ibm.com (8.12.11/8.13.3) with ESMTP id j8K9HTpW005329 for ; Tue, 20 Sep 2005 19:17:29 +1000 Received: from [9.181.133.252] ([9.181.133.252]) by d23av02.au.ibm.com (8.12.11/8.12.11) with ESMTP id j8K9HQ9X005132; Tue, 20 Sep 2005 19:17:27 +1000 Date: Tue, 20 Sep 2005 09:18:00 -0000 From: Wu Zhou To: Daniel Jacobowitz cc: gdb-patches@sources.redhat.com Subject: Re: [RFC] A patch for parse_number (c-exp.y) to recognize 1.25f In-Reply-To: <20050917204405.GB7967@nevyn.them.org> Message-ID: References: <20050917204405.GB7967@nevyn.them.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2005-09/txt/msg00176.txt.bz2 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