public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread

* [patch]
@ 2013-01-23 19:10 Doug Evans
  0 siblings, 0 replies; 7+ messages in thread
From: Doug Evans @ 2013-01-23 19:10 UTC (permalink / raw)
  To: gdb-patches

Hi.

linespec.c:find_linespec_symbols first tries to find "foo::bar" as
method bar in class foo (via lookup_prefix_sym).
If that fails it then tries to find bar in namespace foo.

      /* If successful, we're done.  If NOT_FOUND_ERROR
	 was not thrown, rethrow the exception that we did get.
	 Otherwise, fall back to looking up the entire name as a symbol.
	 This can happen with namespace::function.  */

Given that, there's no need for collect_one_symbol (only used by
lookup_prefix_sym) to collect namespaces.

Regression tested on amd64-linux.
I will commit this patch in a few days if there are no objections.

2013-01-23  Doug Evans  <dje@google.com>

	* linespec.c (collect_one_symbol): Clarify comment.
	Don't collect TYPE_CODE_NAMESPACE symbols.

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.174
diff -u -p -r1.174 linespec.c
--- linespec.c	1 Jan 2013 06:32:46 -0000	1.174
+++ linespec.c	23 Jan 2013 18:36:23 -0000
@@ -2539,7 +2539,7 @@ struct decode_compound_collector
 };
 
 /* A callback for iterate_over_symbols that is used by
-   lookup_prefix_sym to collect type symbols.  */
+   lookup_prefix_sym to collect class symbols.  */
 
 static int
 collect_one_symbol (struct symbol *sym, void *d)
@@ -2554,8 +2554,7 @@ collect_one_symbol (struct symbol *sym, 
   t = SYMBOL_TYPE (sym);
   CHECK_TYPEDEF (t);
   if (TYPE_CODE (t) != TYPE_CODE_STRUCT
-      && TYPE_CODE (t) != TYPE_CODE_UNION
-      && TYPE_CODE (t) != TYPE_CODE_NAMESPACE)
+      && TYPE_CODE (t) != TYPE_CODE_UNION)
     return 1; /* Continue iterating.  */
 
   slot = htab_find_slot (collector->unique_syms, sym, INSERT);

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-01-23 19:10 UTC | newest]

Thread overview: 7+ 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
2013-01-23 19:10 [patch] Doug Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).