From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28794 invoked by alias); 18 Jan 2012 12:48:58 -0000 Received: (qmail 28785 invoked by uid 22791); 18 Jan 2012 12:48:57 -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 12:48:41 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0ICmIvL025239 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 18 Jan 2012 07:48:18 -0500 Received: from psique ([10.3.112.12]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0ICmD5f023102; Wed, 18 Jan 2012 07:48:16 -0500 From: Sergio Durigan Junior To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] References: <20120118045128.GZ31383@adacore.com> <20120118121803.GC31383@adacore.com> X-URL: http://www.redhat.com Date: Wed, 18 Jan 2012 14:34:00 -0000 In-Reply-To: <20120118121803.GC31383@adacore.com> (Joel Brobecker's message of "Wed, 18 Jan 2012 16:18:03 +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/msg00668.txt.bz2 On Wednesday, January 18 2012, Joel Brobecker wrote: >> >> 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. > > Sorry for nit-picking, but since it's very minor and can easily > be deal with... No problem, that's why I sent the patch :-). >> +/* 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. */ > > Suggested re-phrasing: > > /* Helper function to initialize the expout, expout_size, expout_ptr > trio before it is used to store expression elements created during > the parsing of an expression. INITIAL_SIZE is the initial size of > the expout array. LANG is the language used to parse the expression. > And GDBARCH is the gdbarch to use during parsing. */ > >> +/* 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. */ > > Suggest: > > /* Helper function that frees any unsed space in the expout array. > It is generally used when the parser has just been parsed and > created. */ > > (otherwise, remove "the" before "EXPOUT"). > > Ok with those fixes. Fixed and committed the version below. http://sourceware.org/ml/gdb-cvs/2012-01/msg00153.html Thanks. -- 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..32a3bd6 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -182,6 +182,41 @@ free_funcalls (void *ignore) /* This page contains the functions for adding data to the struct expression being constructed. */ +/* Helper function to initialize the expout, expout_size, expout_ptr + trio before it is used to store expression elements created during + the parsing of an expression. INITIAL_SIZE is the initial size of + the expout array. LANG is the language used to parse the expression. + And GDBARCH is the gdbarch to use during parsing. */ + +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 frees any unsed space in the expout array. + It is generally used when the parser 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 +1191,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 +1209,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. */