* [PATCH]
@ 2012-01-18 4:35 Sergio Durigan Junior
2012-01-18 4:52 ` [PATCH] Add initialize and reallocate methods to parser expout Sergio Durigan Junior
2012-01-18 5:04 ` [PATCH] Joel Brobecker
0 siblings, 2 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2012-01-18 4:35 UTC (permalink / raw)
To: gdb-patches
Hi,
I believe this is a rather obvious patch, but I decided to ask anyway.
This is a pretty simple cleanup on parse.c: the creation of
`initialize_expout' and `reallocate_expout' to handle cases when one
needs to do those actions on expout.
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.
This patch also relates to the parser cleanup that I am working on.
Ok to apply?
--
Sergio
2012-01-18 Sergio Durigan Junior <sergiodj@redhat.com>
* 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..b11070c 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 used to initialize an struct expression that is going
+ to be used to store expression elements. The arguments are the
+ parser_state PS containing the EXPOUT, 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 inside PS 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 +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. */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Add initialize and reallocate methods to parser expout
2012-01-18 4:35 [PATCH] Sergio Durigan Junior
@ 2012-01-18 4:52 ` Sergio Durigan Junior
2012-01-18 5:04 ` [PATCH] Joel Brobecker
1 sibling, 0 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2012-01-18 4:52 UTC (permalink / raw)
To: gdb-patches
On Wednesday, January 18 2012, I wrote:
> Hi,
Sorry, I was writing the email's subject and got sidetracked, so I
forgot to specify it correctly.
Sergio.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]
2012-01-18 4:35 [PATCH] Sergio Durigan Junior
2012-01-18 4:52 ` [PATCH] Add initialize and reallocate methods to parser expout Sergio Durigan Junior
@ 2012-01-18 5:04 ` Joel Brobecker
2012-01-18 5:17 ` [PATCH] Sergio Durigan Junior
1 sibling, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2012-01-18 5:04 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> 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 <sergiodj@redhat.com>
>
> * 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.
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?
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]
2012-01-18 5:04 ` [PATCH] Joel Brobecker
@ 2012-01-18 5:17 ` Sergio Durigan Junior
2012-01-18 12:48 ` [PATCH] Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Sergio Durigan Junior @ 2012-01-18 5:17 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
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 <sergiodj@redhat.com>
>>
>> * 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 <sergiodj@redhat.com>
* 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. */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]
2012-01-18 5:17 ` [PATCH] Sergio Durigan Junior
@ 2012-01-18 12:48 ` Joel Brobecker
2012-01-18 14:34 ` [PATCH] Sergio Durigan Junior
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2012-01-18 12:48 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> >> 2012-01-18 Sergio Durigan Junior <sergiodj@redhat.com>
> >>
> >> * 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...
> +/* 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.
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]
2012-01-18 12:48 ` [PATCH] Joel Brobecker
@ 2012-01-18 14:34 ` Sergio Durigan Junior
0 siblings, 0 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2012-01-18 14:34 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Wednesday, January 18 2012, Joel Brobecker wrote:
>> >> 2012-01-18 Sergio Durigan Junior <sergiodj@redhat.com>
>> >>
>> >> * 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 <sergiodj@redhat.com>
* 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. */
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-18 12:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-18 4:35 [PATCH] Sergio Durigan Junior
2012-01-18 4:52 ` [PATCH] Add initialize and reallocate methods to parser expout Sergio Durigan Junior
2012-01-18 5:04 ` [PATCH] Joel Brobecker
2012-01-18 5:17 ` [PATCH] Sergio Durigan Junior
2012-01-18 12:48 ` [PATCH] Joel Brobecker
2012-01-18 14:34 ` [PATCH] Sergio Durigan Junior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox