From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26623 invoked by alias); 12 Nov 2014 15:15:30 -0000 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 Received: (qmail 26594 invoked by uid 89); 12 Nov 2014 15:15:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mga01.intel.com Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Nov 2014 15:15:22 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 12 Nov 2014 07:10:37 -0800 X-ExtLoop1: 1 Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga002.fm.intel.com with ESMTP; 12 Nov 2014 07:10:33 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.58]) by IRSMSX101.ger.corp.intel.com ([169.254.1.144]) with mapi id 14.03.0195.001; Wed, 12 Nov 2014 15:09:24 +0000 From: "Tedeschi, Walfred" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH] In MI mode -var-create --language not working. Date: Wed, 12 Nov 2014 15:15:00 -0000 Message-ID: References: <1412062176-23484-1-git-send-email-walfred.tedeschi@intel.com> <546376A7.8090101@redhat.com> In-Reply-To: <546376A7.8090101@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00214.txt.bz2 Hi Pedro, Thanks a lot for your review!!! I will address all the comments and send to another round. Thanks and regards, -Fred -----Original Message----- From: Pedro Alves [mailto:palves@redhat.com]=20 Sent: Wednesday, November 12, 2014 4:03 PM To: Tedeschi, Walfred; mark.kettenis@xs4all.nl Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] In MI mode -var-create --language not working. Hi Walfred, On 09/30/2014 08:29 AM, Walfred Tedeschi wrote: > Trying to use --language to create a variable showed that --language=20 > was not always working. In some cases GDB understands that the=20 > language is automatic and cannot parse the language set while executing t= he command. > In order to fix this just before executing the command language mode=20 > should be set to manual. In this way GDB can parse the expression=20 > using the language given in the command. > To do so mode has temporarily to be changed to manual expressing that=20 > any evaluation done using the language parameter has priority over the=20 > automatic one. > Tests are also included doing evaluations when the default language is=20 > c/c++ and when default language is Fortran. >=20 > 2014-07-25 Walfred Tedeschi >=20 > * utils.c (saved_language_and_mode): new struct to accommodate > the language mode and the current language for cleanup. > (do_restore_current_language): Add the language and mode > to be restored. (make_cleanup_restore_current_language): > pass the mode and language to the do_restore_current_language. Start sentences with uppercase. Don't explain what the struct is for here.= Line break before "(make_...". "to the do_restore_current_language" sounds a little strange. I suggest this: * utils.c (struct saved_language_and_mode): New. (do_restore_current_language): Add the language and mode to be restored. (make_cleanup_restore_current_language): Pass the mode and language to do_restore_current_language. >=20 > gdb/mi > * mi-main.c (mi_cmd_execute): Set to manual the language mode > to execute an mi command when --language is present. * mi-main.c (mi_cmd_execute): When --language is present, set the language mode to manual. > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index=20 > 59717ca..0198981 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -2236,6 +2236,7 @@ mi_cmd_execute (struct mi_parse *parse) > if (parse->language !=3D language_unknown) > { > make_cleanup_restore_current_language (); > + language_mode =3D language_mode_manual; > set_language (parse->language); > } >=20=20 Did you audit all set_language callers, to see what other callers might nee= d the same fix? It might be better to add a variant of set_language that d= oes the forcing of language_mode to manual. Say, something like set_langua= ge_manual, or override_language, or some such, and call that in all places = that might need this. > +set lineno [gdb_get_line_number "only bp."] > + > +mi_create_breakpoint "$srcfile:$lineno" "add mi-language1 bp" keep {main= \(\)} \ > + ".*mi-language1.cc" $lineno $hex "break in main" Use $srcfile instead of "mi-language1.cc". "break in main" seems stale? > + > +mi_execute_to "exec-continue" "breakpoint-hit" \ > + "main" "" ".*" ".*" {"" "disp=3D\"keep\""} \ > + "continue to mi-language1 bp" No need to say "mi-language1" here. The test messages are always prefixed = with the test file name. > + > +gdb_exit > \ No newline at end of file Please make sure new files end with a new line. > diff --git a/gdb/testsuite/gdb.mi/mi-language2.exp=20 > b/gdb/testsuite/gdb.mi/mi-language2.exp How about renaming these to mi-language-c.exp, mi-language-fortran.exp ? mi-language1.exp and mi-language2.exp seem a bit arbitrary. > +set bp_lineno [gdb_get_line_number "only bp"] > + > +mi_create_breakpoint "-t mi-language2.f90:$bp_lineno" "add=20 > +mi-language2 bp"\ > + "del" "struct_test" ".*mi-language2.f90" $bp_lineno $hex \ > + "MI-language2 insert breakpoint at line $bp_lineno (only bp)" > + No need for this "MI-language2" prefix. Please don't put line numbers in t= he test message, as those will change when we add something to the test sou= rce. Write instead: "insert breakpoint at only bp" or some such. > +mi_run_cmd > + > +mi_expect_stop "breakpoint-hit" "struct_test" "" ".*mi-language2.f90" \ > + "$bp_lineno" { "" "disp=3D\"del\"" } "mi-language2 run to= breakpoint at line $bp_lineno" > + > +mi_gdb_test "-var-create --language c alpha_1 * (p.c)" \ > + "\\^done,name=3D\"alpha_1\",numchild=3D\"0\",value=3D\"1\",t= ype=3D\"integer\\(kind=3D4\\)\",thread-id=3D\"\[0-9\]+\",has_more=3D\"0\"" \ > + "-var-create from fortran using fortran language" > + > +mi_gdb_test "-var-create alpha_2 * (p%c)" \ > + "\\^done,name=3D\"alpha_2\",numchild=3D\"0\",value=3D\"1\",t= ype=3D\"integer\\(kind=3D4\\)\",thread-id=3D\"\[0-9\]+\",has_more=3D\"0\"" \ > + "-var-create from fortran using default language" > +gdb_exit > \ No newline at end of file Add a newline. > diff --git a/gdb/testsuite/gdb.mi/mi-language2.f90=20 > b/gdb/testsuite/gdb.mi/mi-language2.f90 > new file mode 100644 > index 0000000..ec49f2b > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-language2.f90 > @@ -0,0 +1,40 @@ > +! Copyright 2014 Free Software Foundation, Inc. ... > +! > +! Ihis file is the Fortran source file for derived-type.exp. It was=20 > +written ! by Wu Zhou. (woodzltc@cn.ibm.com) This reference to derived-type.exp is confusing now. I think it's best to = just remove the whole paragraph. Also, if this file is mostly copied from = another, please preserve the copyright years of the original file. > diff --git a/gdb/utils.c b/gdb/utils.c index 4da9501..96a87ca 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -444,24 +444,38 @@ make_cleanup_free_so (struct so_list *so) >=20=20 > /* Helper for make_cleanup_restore_current_language. */ >=20=20 > +struct saved_language_and_mode > +{ > + enum language lang; > + enum language_mode mode; > +}; > + > static void > do_restore_current_language (void *p) { > - enum language saved_lang =3D (uintptr_t) p; > + struct saved_language_and_mode *lang_and_mode =3D > + (struct saved_language_and_mode*) p; struct saved_language_and_mode *lang_and_mode =3D (struct saved_language_and_mode*) p; Though in C you don't really need the cast. > + enum language saved_lang =3D lang_and_mode->lang; language_mode =3D=20 > + lang_and_mode->mode; >=20=20 > + xfree (p); This xfree should be done in the cleanup's destructor, so that we don't lea= k p when discard_cleanups is called. That is, use make_cleanup_dtor instea= d of make_cleanup in make_cleanup_restore_current_language. > set_language (saved_lang); > } >=20=20 > -/* Remember the current value of CURRENT_LANGUAGE and make it restored w= hen > - the cleanup is run. */ > +/* Remember the current value of CURRENT_LANGUAGE and > + LANGUAGE_MODE restoring them when the cleanup is run. */ >=20=20 > struct cleanup * > make_cleanup_restore_current_language (void) { > - enum language saved_lang =3D current_language->la_language; > + struct saved_language_and_mode *lang_and_mode > + =3D XNEW (struct saved_language_and_mode); Indentation: struct saved_language_and_mode *lang_and_mode =3D XNEW (struct saved_language_and_mode); Thanks, Pedro Alves Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052