From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1652 invoked by alias); 18 Jan 2012 05:04:30 -0000 Received: (qmail 1642 invoked by uid 22791); 18 Jan 2012 05:04:27 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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; Wed, 18 Jan 2012 05:04:11 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0I53n3M011148 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 18 Jan 2012 00:03:49 -0500 Received: from psique ([10.3.112.12]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0I53iGh021942; Wed, 18 Jan 2012 00:03:46 -0500 From: Sergio Durigan Junior To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] References: <20120118045128.GZ31383@adacore.com> X-URL: http://www.redhat.com Date: Wed, 18 Jan 2012 05:17:00 -0000 In-Reply-To: <20120118045128.GZ31383@adacore.com> (Joel Brobecker's message of "Wed, 18 Jan 2012 08:51:28 +0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2012-01/txt/msg00662.txt.bz2 Hi Joel, Thank for the review. On Wednesday, January 18 2012, Joel Brobecker wrote: >> This patch is going to be important for the coming SystemTap patches >> that I plan to submit this week, and I know GDB has a rule to avoid >> premature modification of the code before a patch is posted, but I >> believe this is a good cleanup to have in any case. > [...] >> 2012-01-18 Sergio Durigan Junior >> >> * parse.c (initialize_expout): New function. >> (reallocate_expout): Likewise. >> (parse_exp_in_context): Use `initialize_expout' and >> `reallocate_expout' when appropriate. > > FWIW, it seems OK to me on its own. I like the fact that you made > the initial_size and gdbarch parameters. Thanks. > The description does seem to mention an argument that has not been > added yet (parser_state PS). I haven't followed the discussion on > the thread, but have we decided to keep "ps" as the name of the > parser_state argument? Sorry, this was actually another mistake. I am getting crazy with so many parser patches. This argument is not present yet because the parser cleanup is an ongoing work, so I will remove its mention on the comment. As for your question about the name: Tom asked me to change the name to something else, so it's not going to be `ps' anymore. But that is probably a discussion for the other thread :-). Anyway, here is the fixed version of the patch. -- Sergio 2012-01-18 Sergio Durigan Junior * parse.c (initialize_expout): New function. (reallocate_expout): Likewise. (parse_exp_in_context): Use `initialize_expout' and `reallocate_expout' when appropriate. diff --git a/gdb/parse.c b/gdb/parse.c index f405dc5..1ce3b4b 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -182,6 +182,40 @@ free_funcalls (void *ignore) /* This page contains the functions for adding data to the struct expression being constructed. */ +/* Helper function used to initialize an struct expression that is going + to be used to store expression elements. The arguments are the + INITIAL_SIZE of the expression, the language LANG parsed to build this + expression, and the GDBARCH pointer. */ + +static void +initialize_expout (int initial_size, const struct language_defn *lang, + struct gdbarch *gdbarch) +{ + expout_size = initial_size; + expout_ptr = 0; + expout = xmalloc (sizeof (struct expression) + + EXP_ELEM_TO_BYTES (expout_size)); + expout->language_defn = lang; + expout->gdbarch = gdbarch; +} + +/* Helper function that reallocates the EXPOUT in order to eliminate any + unused space. It is generally used when the expression has just been + parsed and created. */ + +static void +reallocate_expout (void) +{ + /* Record the actual number of expression elements, and then + reallocate the expression memory so that we free up any + excess elements. */ + + expout->nelts = expout_ptr; + expout = xrealloc ((char *) expout, + sizeof (struct expression) + + EXP_ELEM_TO_BYTES (expout_ptr)); +} + /* Add one element to the end of the expression. */ /* To avoid a bug in the Sun 4 compiler, we pass things that can fit into @@ -1156,12 +1190,7 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma, else lang = current_language; - expout_size = 10; - expout_ptr = 0; - expout = (struct expression *) - xmalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_size)); - expout->language_defn = lang; - expout->gdbarch = get_current_arch (); + initialize_expout (10, lang, get_current_arch ()); TRY_CATCH (except, RETURN_MASK_ALL) { @@ -1179,14 +1208,7 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma, discard_cleanups (old_chain); - /* Record the actual number of expression elements, and then - reallocate the expression memory so that we free up any - excess elements. */ - - expout->nelts = expout_ptr; - expout = (struct expression *) - xrealloc ((char *) expout, - sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_ptr)); + reallocate_expout (); /* Convert expression from postfix form as generated by yacc parser, to a prefix form. */