From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28259 invoked by alias); 13 Feb 2010 11:49:44 -0000 Received: (qmail 28250 invoked by uid 22791); 13 Feb 2010 11:49:43 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 13 Feb 2010 11:49:38 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o1DBnbHr014782 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 13 Feb 2010 06:49:37 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o1DBnYYm003358 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 13 Feb 2010 06:49:36 -0500 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id o1DBnYUK001935; Sat, 13 Feb 2010 12:49:34 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id o1DBnYAD001934; Sat, 13 Feb 2010 12:49:34 +0100 Date: Sat, 13 Feb 2010 11:49:00 -0000 From: Jan Kratochvil To: Chris Moller Cc: gdb-patches@sourceware.org Subject: Re: pr 11067 patch Message-ID: <20100213114933.GA595@host0.dyn.jankratochvil.net> References: <4B737180.4050802@redhat.com> <20100211092950.GC2907@adacore.com> <20100212041137.GE2907@adacore.com> <4B75783D.6050103@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B75783D.6050103@redhat.com> User-Agent: Mutt/1.5.20 (2009-08-17) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-02/txt/msg00333.txt.bz2 Hi Chris, On Fri, 12 Feb 2010 16:48:13 +0100, Chris Moller wrote: > --- gdb/testsuite/gdb.mi/mi-var-display.exp 1 Jan 2010 07:32:03 -0000 1.34 > +++ gdb/testsuite/gdb.mi/mi-var-display.exp 12 Feb 2010 15:38:07 -0000 > mi_gdb_test "-var-evaluate-expression anone" \ > - "\\^done,value=\"A\"" \ > + "\\^done,value=\"A = \\(enum \\)0\"" \ > "eval variable anone" While it works with Eclipse should this format be used even for MI? MI doc says: returns its value as a string. It is true it may be the same what `print' doc says: By default, GDB prints a value according to its data type. Still IMO the representation "A = (enum )0" is not suitable for MI as it is not the intended type of a C expression. But in practice for Eclipse it works and it brings better user experience - as in CLI. Some GNU Coding Standard nitpicks: > Index: gdb/ChangeLog$ > +Wed Feb 10 17:13:44 2010 Chris Moller timestamp line does not follow GCS. > + /* > + When printing a simple enum value, the following code includes, in > + addition to the symbolic value, the numeric value and the enum tag. > + Under other-than-simple circumstances--in structs, arrays, etc.-- > + it does what's always done and prints just the symbolic value. > + */ GCS comment should be formatted: /* text... text. */ > + else if (TYPE_TAG_NAME(type)) > + enum_name = TYPE_TAG_NAME(type); GCS spacing: -> "TYPE_TAG_NAME (type)" > + else > + enum_name = ""; > + = +^I $ git diff --check says: gdb/c-valprint.c:433: trailing whitespace. (but unaware how to check an already axisting .patch file) > + fprintf_filtered (stream, "%s = (enum %s)%s", -> # + fprintf_filtered (stream, "%s = (enum %s) %s", while PR 11067 shows it that way GCS 5.3 suggests a whitespace there: foo = (char *) malloc (sizeof *foo); > +++ gdb/testsuite/gdb.base/pr11067.c 12 Feb 2010 15:38:05 -0000 > @@ -0,0 +1,48 @@ > +enum E { > + Val1 = 56, > + Val2 > +}; According to GNU indent it should be: enum E { Val1 = 56, Val2 }; > +struct Es { ditto > +int main() { GCS: int main () { > + /* > + The following is a fake-out for linkers (such as, apparently, the AIX > + linker) that optimises out unreference variables. > + */ Comment -> GCS. > + return 0; > +} > + trailing whitespace line. > +++ gdb/testsuite/gdb.base/pr11067.exp 12 Feb 2010 15:38:05 -0000 > +gdb_test "print e" "Val1 = \\(enum E\\)56" > + > +gdb_test "print ea" "{Val1, Val2, Val1}" > + Two spaces. GDB testscases use more a practice to expect also leading " = ": gdb_test "print e" " = Val1 = \\(enum E\\)56" gdb_test "print ea" " = {Val1, Val2, Val1}" > +gdb_test "print es" "v = 5, e = Val2}" + IMO missing opening bracket. gdb_test "print es" " = {v = 5, e = Val2}" Regards, Jan