From: Daniel Jacobowitz <drow@false.org>
To: Mike Frysinger <vapier@gentoo.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch ping] change lookup order of $localvars to happen before symbol tables
Date: Sat, 16 Sep 2006 04:02:00 -0000 [thread overview]
Message-ID: <20060916040222.GA7673@nevyn.them.org> (raw)
In-Reply-To: <200609140100.54614.vapier@gentoo.org>
Sorry, I'm way behind on patches, and it seems others don't have
much time for review either.
On Thu, Sep 14, 2006 at 01:00:54AM -0400, Mike Frysinger wrote:
> 2006-08-21 Mike Frysinger <vapier@gentoo.org>
>
> * value.h (lookup_only_internalvar): New prototype.
> * value.c (lookup_only_internalvar): New function.
> (lookup_internalvar): Use lookup_only_internalvar.
> * parse.c (write_dollar_variable): Look up $ symbols in internal
> table first rather than last.
I think this approach is basically OK. Minor comments on the
patch:
> --- gdb/parse.c
> +++ gdb/parse.c
> @@ -486,6 +486,17 @@ write_dollar_variable (struct stoken str
> if (i >= 0)
> goto handle_register;
>
> + /* Any names starting with $ are probably debugger internal variables. */
> +
> + if (str.ptr[0] == '$' && lookup_only_internalvar (copy_name (str) + 1))
> + {
> +internal_lookup:
> + write_exp_elt_opcode (OP_INTERNALVAR);
> + write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
> + write_exp_elt_opcode (OP_INTERNALVAR);
> + return;
> + }
You don't need to check ptr[0]; this is write_dollar_variable, if it
didn't start with a dollar sign we wouldn't be here :-)
Scanning the list of internal variables is linear. Let's not be
wasteful with it; please save the result the first time you do it.
> @@ -508,12 +519,9 @@ write_dollar_variable (struct stoken str
> return;
> }
>
> - /* Any other names starting in $ are debugger internal variables. */
> -
> - write_exp_elt_opcode (OP_INTERNALVAR);
> - write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
> - write_exp_elt_opcode (OP_INTERNALVAR);
> - return;
> + /* Any other names are assumed to be debugger internal variables. */
> +
> + goto internal_lookup;
Let's avoid goto, please, since it's only three lines. We're going
to rescan the internal variable list here, but we don't much care.
Would you mind resubmitting with those changes?
--
Daniel Jacobowitz
CodeSourcery
next prev parent reply other threads:[~2006-09-16 4:02 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-08-20 14:28 only force symbol lookups with local variables on hppa Mike Frysinger
2006-08-20 14:33 ` Mark Kettenis
2006-08-20 14:38 ` Daniel Jacobowitz
2006-08-22 1:35 ` Mark Kettenis
2006-08-22 2:06 ` Daniel Jacobowitz
2006-08-22 18:01 ` Mike Frysinger
2006-08-22 18:21 ` Daniel Jacobowitz
2006-08-22 18:34 ` Mike Frysinger
2006-09-14 5:01 ` [patch ping] change lookup order of $localvars to happen before symbol tables Mike Frysinger
2006-09-16 4:02 ` Daniel Jacobowitz [this message]
2006-09-17 4:22 ` Mike Frysinger
2006-09-17 15:10 ` Daniel Jacobowitz
2006-09-17 20:22 ` Mike Frysinger
2006-09-17 20:28 ` Daniel Jacobowitz
2006-09-27 5:51 ` Mike Frysinger
2007-09-26 19:50 ` Mike Frysinger
2007-10-01 0:37 ` Daniel Jacobowitz
2007-10-01 1:41 ` Mike Frysinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060916040222.GA7673@nevyn.them.org \
--to=drow@false.org \
--cc=gdb-patches@sourceware.org \
--cc=vapier@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox