Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Vladimir Prus <ghost@cs.msu.su>
Cc: gdb-patches@sources.redhat.com
Subject: Re: -var-info-path-expression
Date: Mon, 29 Jan 2007 12:17:00 -0000	[thread overview]
Message-ID: <20070129121648.GA25075@nevyn.them.org> (raw)
In-Reply-To: <200701280048.37153.ghost@cs.msu.su>

On Sun, Jan 28, 2007 at 12:48:36AM +0300, Vladimir Prus wrote:
> I guess I do know -- I don't care about minor interface details. It's more
> important to have this implemented than solving "attribute vs. command"
> question the right way, and therefore, using a separate command is fine.
> 
> Do you want me to add docs/tests or you can review the current 
> version of the patch, reposted here fore convenience?

I'll just review this copy.  I think having it pop out of
-var-list-children automatically would be useful, but the testsuite
updates would be a pain - maybe we should keep that in mind when
we add tests for new MI commands and try to use more functions.

> +  if (argc != 1)
> +    error ("mi_cmd_var_info_path_expression: Usage: NAME.");
> +
> +  /* Get varobj handle, if a valid var obj name was specified */
> +  var = varobj_get_handle (argv[0]);
> +  if (var == NULL)
> +    error ("mi_cmd_var_info_path_expression: Variable object not found");

Missing _().  Also, we didn't update existing commands, but I think we
decided the "function:" prefixes weren't helpful.

> @@ -757,6 +779,21 @@ varobj_get_gdb_type (struct varobj *var)
>    return var->type;
>  }
>  
> +/* Return a pointer to the full rooted expression of varobj VAR.
> +   If it has not been computed yet, compute it */
> +char *
> +varobj_get_path_expr (struct varobj *var)
> +{
> +  if (var->path_expr != NULL)
> +    return var->path_expr;
> +  else if (is_root_p (var))
> +    return var->name;
> +  else
> +    {
> +      return (*var->root->lang->path_expr_of_child) (var);
> +    }
> +}

Since you initialize path_expr at the same time as name, will is_root_p
ever trigger here?

> @@ -1826,10 +1875,13 @@ value_struct_element_index (struct value
>     to NULL.  */
>  static void 
>  c_describe_child (struct varobj *parent, int index,
> -		  char **cname, struct value **cvalue, struct type **ctype)
> +		  char **cname, struct value **cvalue, struct type **ctype,
> +		  char **cfull_expression)

Nick's got a point about the growing number of arguments.  Would
converting them to a struct simplify it?

struct varobj_child_desc
{
  char *name;
  struct value *value;
  struct type *type;
  char *full_expression;
};

Hey... those fields all live in struct varobj... I wonder if this code
ought to be rearranged so that this initializes the child's struct
varobj.  But anyway let's not do that right now.  The new argument is
fine, I was just thinking out loud.

-- 
Daniel Jacobowitz
CodeSourcery


  parent reply	other threads:[~2007-01-29 12:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-25  9:02 -var-info-path-expression Vladimir Prus
2007-01-03 22:39 ` -var-info-path-expression Daniel Jacobowitz
2007-01-05  9:14   ` -var-info-path-expression Vladimir Prus
2007-01-27 21:49     ` -var-info-path-expression Vladimir Prus
2007-01-28  0:46       ` -var-info-path-expression Nick Roberts
2007-01-28  7:57         ` -var-info-path-expression Vladimir Prus
2007-01-28  8:35           ` -var-info-path-expression Nick Roberts
2007-01-29 10:53             ` -var-info-path-expression Vladimir Prus
2007-01-29 12:17       ` Daniel Jacobowitz [this message]
2007-06-18 10:43 -var-info-path-expression Vladimir Prus
2007-06-18 19:40 ` -var-info-path-expression Eli Zaretskii
2007-06-21  5:15   ` -var-info-path-expression Vladimir Prus
2007-07-03 17:14 ` -var-info-path-expression Daniel Jacobowitz
2007-07-03 17:51   ` -var-info-path-expression Vladimir Prus
2007-07-03 17:58     ` -var-info-path-expression Daniel Jacobowitz
2007-08-22 11:23 -var-info-path-expression Vladimir Prus
2007-08-22 15:28 ` -var-info-path-expression Daniel Jacobowitz
2007-08-28 17:18 ` -var-info-path-expression Vladimir Prus
2007-08-28 19:55   ` -var-info-path-expression Eli Zaretskii
2007-08-31  9:08   ` -var-info-path-expression Eli Zaretskii
2007-08-31  9:57     ` -var-info-path-expression Vladimir Prus
2007-08-31 18:04       ` -var-info-path-expression Eli Zaretskii
2007-08-31 18:53         ` -var-info-path-expression Vladimir Prus
2007-09-01  7:56           ` -var-info-path-expression Eli Zaretskii

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=20070129121648.GA25075@nevyn.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=ghost@cs.msu.su \
    /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