From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15770 invoked by alias); 17 Sep 2005 20:44:12 -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 15758 invoked by uid 22791); 17 Sep 2005 20:44:08 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sat, 17 Sep 2005 20:44:08 +0000 Received: from drow by nevyn.them.org with local (Exim 4.52) id 1EGjXZ-0002HV-Hh; Sat, 17 Sep 2005 16:44:05 -0400 Date: Sat, 17 Sep 2005 20:44:00 -0000 From: Daniel Jacobowitz To: Wu Zhou Cc: gdb-patches@sources.redhat.com Subject: Re: [RFC] A patch for parse_number (c-exp.y) to recognize 1.25f Message-ID: <20050917204405.GB7967@nevyn.them.org> Mail-Followup-To: Wu Zhou , gdb-patches@sources.redhat.com References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.8i X-SW-Source: 2005-09/txt/msg00120.txt.bz2 On Thu, Sep 15, 2005 at 04:05:48PM +0800, Wu Zhou wrote: > > > To fix this kind of false-negative and not to introduce false-positive, I > > am thinking of changing the second format in sscanf to "%s" (string). > > Thus the parser could get the trailing characters following the float. > > Then we can add some code to parse them to eliminate false-positive. Any > > points on this idea? Is it worthwhile to do this? Please comment. TIA. > > Following the above point, I coded another patch. It can fix the problem > and don't introduce any false-positive mentioned above. I had also coded > a testcase for that. Had tested on i386. With the patched GDB, it > reported 18 PASS; with the original GDB, it report 9 PASS and 9 FAIL (all > of these failure is due to the fact that GDB can't handle the suffix > following the float constants.) > > IMHO, there are yet another advantage for this patch: it is easy to be > extended to handle other suffixes, such "df" for _Decimal32, "dd" for > _Decimal64 and "dl" for _Decimal128, all of which are described in IEEE > 754R for decimal floating point. > > Here goes the patch and the testcase. This looks good. I have some comments on formatting and testing, but that's it. > 2005-09-15 Wu Zhou > > * c-exp.y (parse-number): Modify the float parsing logic to let it > recognize the suffix. "a suffix" rather than "the suffix", please. > ! 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. > Index: gdb.base/bfp-test.c > =================================================================== > RCS file: gdb.base/bfp-test.c > diff -N gdb.base/bfp-test.c > *** /dev/null 1 Jan 1970 00:00:00 -0000 > --- gdb.base/bfp-test.c 15 Sep 2005 06:49:51 -0000 > *************** > *** 0 **** > --- 1,15 ---- It's a pretty trivial test case; but please add a copyright notice anyway. > + # 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. > + # 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. gdb_test_multiple should work OK everywhere you used send_gdb. > + # Test that gdb could handle invalid suffix correctly. > + send_gdb "set var b32=100.5a\n" > + gdb_expect { > + -re "$gdb_prompt $" { fail "don't report error on 100.5a" } > + -re "Invalid number" { pass "100.5a is invalid number" } > + timeout {fail "set var b32=100.5a" } > + } ... although all of the invalid tests could just use gdb_test. -- Daniel Jacobowitz CodeSourcery, LLC