Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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


  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