Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] "constify" parse_exp_1
@ 2013-03-08  0:12 Keith Seitz
  2013-03-08  0:22 ` Pedro Alves
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2013-03-08  0:12 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

Hi,

This is the follow-on patch that I mentioned yesterday. Most of the 
changes are pretty mechanical. I would appreciate another pair of eagle 
eyes, though.

This patch "constifies" parse_exp_1 by making it argument const and 
strdup'ing it to rigidly enforce this. As Tom points out, this is 
necessary because nearly all of the language parsers *will* temporarily 
modify the input string. I have some patches for this that I will post 
next week.

I've tested this by rebuilding with --enable-targets=all on x86_64 
linux. There are no regressions against CVS HEAD on native, 
native-gdbserver, or cc-with-tweaks(-i).

This patch has the added side effect of constifying 
find_condition_and_thread, which is really what I've been after all 
along. :-)

Comments/questions/concerns?
Keith

ChangeLog
2013-03-07  Keith Seitz  <keiths@redhat.com>

	* ada-lang.c (ada_read_renaming_var_value): Pass const
	pointer to expression string to parse_exp_1.
	(craete_excep_cond_exprs): Likewise.
	* ax-gdb.c (agent_eval_command_one): Likewise.
	(maint_agent_printf_command): Likewise.
	Constify much of the string handling/parsing.
	* breakpoint.c (set_breakpoint_condition): Pass const
	pointer to expression string to parse_exp_1.
	(update_watchpoint): Likewise.
	(parse_cmd_to_aexpr): Constify string handling.
	Pass const pointer to parse_exp_1.
	(init_breakpoint_sal): Pass const pointer to parse_exp_1.
	(find_condition_and_thread): Likewise.
	Make TOK const.
	(watch_command_1): Constify string handling.
	(update_breakpoint_location): Pass const pointer to
	parse_exp_1.
	* eval.c (parse_and_eval_address): Make EXP const.
	(parse_to_comma_and_eval): Make EXPP const.
	* expression.h (parse_expression): Make argument const.
	(parse_exp_1): Make first argument const.
	* findcmd.c (parse_find_args): Treat ARGS as const.
	* linespec.c (parse_linespec): Pass const pointer to
	linespec_expression_to_pc.
	(linespec_expression_to_pc): Make EXP_PTR const.
	* parse.c (parse_exp_1): Make STRINGPTR const.
	Make a copy of the expression to pass to parse_exp_in_context until
	this whole interface can be constified.
	(parse_expression): Make STRING const.
	* printcmd.c (ui_printf): Treat ARG as const.
	Handle const strings.
	* tracepoint.c (validate_actionline): Pass const pointer to
	all calls to parse_exp_1.
	(encode_actions_1): Likewise.
	* value.h (parse_to_comma_and_eval): Make argument const.
	(parse_and_eval_address): Likewise.
	* varobj.c (varobj_create): Pass const pointer to parse_exp_1.
	(varobj_set_value): Likewise.
	* cli/cli-cmds.c (disassemble_command): Treat ARG as const and
	constify string handling.
	Pass const pointers to parse_and_eval_address and
	parse_to_comman_and_eval.
	* cli/cli-utils.c (skip_to_space): Rename to ...
	(skip_to_space_const): ... this. Handle const strings.
	* cli/cli-utils.h (skip_to_space): Turn into macro which invokes
	skip_to_space_const.
	(skip_to_space_const): Declare.
	* common/format.c (parse_format_string): Make ARG const.
	Handle const strings.
	* common/format.h (parse_format_string): Make ARG const.
	* gdbserver/ax.c (ax_printf): Make FORMAT const.

[-- Attachment #2: constify-parse_exp_1.patch --]
[-- Type: text/x-patch, Size: 26938 bytes --]

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.394
diff -u -p -r1.394 ada-lang.c
--- ada-lang.c	5 Mar 2013 21:15:34 -0000	1.394
+++ ada-lang.c	7 Mar 2013 22:59:29 -0000
@@ -4057,13 +4057,14 @@ ada_read_renaming_var_value (struct symb
 			     struct block *block)
 {
   char *sym_name;
+  const char *sname;
   struct expression *expr;
   struct value *value;
   struct cleanup *old_chain = NULL;
 
-  sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
+  sname = sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
   old_chain = make_cleanup (xfree, sym_name);
-  expr = parse_exp_1 (&sym_name, 0, block, 0);
+  expr = parse_exp_1 (&sname, 0, block, 0);
   make_cleanup (free_current_contents, &expr);
   value = evaluate_expression (expr);
 
@@ -11385,7 +11386,7 @@ create_excep_cond_exprs (struct ada_catc
       if (!bl->shlib_disabled)
 	{
 	  volatile struct gdb_exception e;
-	  char *s;
+	  const char *s;
 
 	  s = cond_string;
 	  TRY_CATCH (e, RETURN_MASK_ERROR)
Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.109
diff -u -p -r1.109 ax-gdb.c
--- ax-gdb.c	7 Mar 2013 00:48:25 -0000	1.109
+++ ax-gdb.c	7 Mar 2013 22:59:29 -0000
@@ -2608,6 +2608,7 @@ agent_eval_command_one (char *exp, int e
   struct cleanup *old_chain = 0;
   struct expression *expr;
   struct agent_expr *agent;
+  const char *arg;
 
   if (!eval)
     {
@@ -2616,14 +2617,15 @@ agent_eval_command_one (char *exp, int e
         exp = decode_agent_options (exp);
     }
 
-  if (!eval && strcmp (exp, "$_ret") == 0)
+  arg = exp;
+  if (!eval && strcmp (arg, "$_ret") == 0)
     {
       agent = gen_trace_for_return_address (pc, get_current_arch ());
       old_chain = make_cleanup_free_agent_expr (agent);
     }
   else
     {
-      expr = parse_exp_1 (&exp, pc, block_for_pc (pc), 0);
+      expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
       old_chain = make_cleanup (free_current_contents, &expr);
       if (eval)
 	agent = gen_eval_for_expr (pc, expr);
@@ -2716,8 +2718,8 @@ maint_agent_printf_command (char *exp, i
   struct expression *argvec[100];
   struct agent_expr *agent;
   struct frame_info *fi = get_current_frame ();	/* need current scope */
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
 
@@ -2733,7 +2735,7 @@ maint_agent_printf_command (char *exp, i
 
   cmdrest = exp;
 
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("Must start with a format string."));
@@ -2749,19 +2751,19 @@ maint_agent_printf_command (char *exp, i
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest != ',' && *cmdrest != 0)
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.744
diff -u -p -r1.744 breakpoint.c
--- breakpoint.c	7 Mar 2013 21:57:29 -0000	1.744
+++ breakpoint.c	7 Mar 2013 22:59:29 -0000
@@ -950,7 +950,7 @@ set_breakpoint_condition (struct breakpo
     }
   else
     {
-      char *arg = exp;
+      const char *arg = exp;
 
       /* I don't know if it matters whether this is the string the user
 	 typed in or the decompiled expression.  */
@@ -1759,7 +1759,7 @@ update_watchpoint (struct watchpoint *b,
 
   if (within_current_scope && reparse)
     {
-      char *s;
+      const char *s;
 
       if (b->exp)
 	{
@@ -2186,8 +2186,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   struct agent_expr *aexpr = NULL;
   struct cleanup *old_chain = NULL;
   volatile struct gdb_exception ex;
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
   struct gdbarch *gdbarch = get_current_arch ();
@@ -2199,7 +2199,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
 
   if (*cmdrest == ',')
     ++cmdrest;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("No format string following the location"));
@@ -2215,14 +2215,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (!(*cmdrest == ',' || *cmdrest == '\0'))
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   /* For each argument, make an expression.  */
 
@@ -2232,7 +2232,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
@@ -9103,7 +9103,7 @@ init_breakpoint_sal (struct breakpoint *
 
       if (b->cond_string)
 	{
-	  char *arg = b->cond_string;
+	  const char *arg = b->cond_string;
 	  loc->cond = parse_exp_1 (&arg, loc->address,
 				   block_for_pc (loc->address), 0);
 	  if (*arg)
@@ -9374,7 +9374,7 @@ invalid_thread_id_error (int id)
    If no thread is found, *THREAD is set to -1.  */
 
 static void
-find_condition_and_thread (char *tok, CORE_ADDR pc,
+find_condition_and_thread (const char *tok, CORE_ADDR pc,
 			   char **cond_string, int *thread, int *task,
 			   char **rest)
 {
@@ -9385,12 +9385,12 @@ find_condition_and_thread (char *tok, CO
 
   while (tok && *tok)
     {
-      char *end_tok;
+      const char *end_tok;
       int toklen;
-      char *cond_start = NULL;
-      char *cond_end = NULL;
+      const char *cond_start = NULL;
+      const char *cond_end = NULL;
 
-      tok = skip_spaces (tok);
+      tok = skip_spaces_const (tok);
 
       if ((*tok == '"' || *tok == ',') && rest)
 	{
@@ -9398,7 +9398,7 @@ find_condition_and_thread (char *tok, CO
 	  return;
 	}
 
-      end_tok = skip_to_space (tok);
+      end_tok = skip_to_space_const (tok);
 
       toklen = end_tok - tok;
 
@@ -9414,11 +9414,13 @@ find_condition_and_thread (char *tok, CO
 	}
       else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
 	{
-	  char *tmptok;
+	  char *endp;
+	  const char *tmptok;
 
 	  tok = end_tok + 1;
 	  tmptok = tok;
-	  *thread = strtol (tok, &tok, 0);
+	  *thread = strtol (tok, &endp, 0);
+	  tok = endp;
 	  if (tok == tmptok)
 	    error (_("Junk after thread keyword."));
 	  if (!valid_thread_id (*thread))
@@ -9426,11 +9428,13 @@ find_condition_and_thread (char *tok, CO
 	}
       else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
 	{
-	  char *tmptok;
+	  char *endp;
+	  const char *tmptok;
 
 	  tok = end_tok + 1;
 	  tmptok = tok;
-	  *task = strtol (tok, &tok, 0);
+	  *task = strtol (tok, &endp, 0);
+	  tok = endp;
 	  if (tok == tmptok)
 	    error (_("Junk after task keyword."));
 	  if (!valid_task_id (*task))
@@ -10845,12 +10849,12 @@ watch_command_1 (char *arg, int accessfl
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   struct frame_info *frame;
-  char *exp_start = NULL;
-  char *exp_end = NULL;
-  char *tok, *end_tok;
+  const char *exp_start = NULL;
+  const char *exp_end = NULL;
+  const char *tok, *end_tok;
   int toklen = -1;
-  char *cond_start = NULL;
-  char *cond_end = NULL;
+  const char *cond_start = NULL;
+  const char *cond_end = NULL;
   enum bptype bp_type;
   int thread = -1;
   int pc = 0;
@@ -10859,15 +10863,19 @@ watch_command_1 (char *arg, int accessfl
   int use_mask = 0;
   CORE_ADDR mask = 0;
   struct watchpoint *w;
+  const char *tmp, *start, *end_arg, *cbuf;
+  char *buf;
 
   /* Make sure that we actually have parameters to parse.  */
   if (arg != NULL && arg[0] != '\0')
     {
-      char *value_start;
+      const char *value_start;
+
+      end_arg = arg + strlen (arg);
 
       /* Look for "parameter value" pairs at the end
 	 of the arguments string.  */
-      for (tok = arg + strlen (arg) - 1; tok > arg; tok--)
+      for (tok = end_arg - 1; tok > arg; tok--)
 	{
 	  /* Skip whitespace at the end of the argument list.  */
 	  while (tok > arg && (*tok == ' ' || *tok == '\t'))
@@ -10937,15 +10945,22 @@ watch_command_1 (char *arg, int accessfl
 
 	  /* Truncate the string and get rid of the "parameter value" pair before
 	     the arguments string is parsed by the parse_exp_1 function.  */
-	  *tok = '\0';
+	  end_arg = tok;
 	}
     }
+  else
+    end_arg = arg;
 
   /* Parse the rest of the arguments.  */
   innermost_block = NULL;
-  exp_start = arg;
-  exp = parse_exp_1 (&arg, 0, 0, 0);
-  exp_end = arg;
+  buf = alloca (end_arg - arg + 1);
+  strncpy (buf, arg, end_arg - arg);
+  buf[end_arg - arg] = '\0';
+  exp_start = buf;
+  tmp = start = buf;
+  exp = parse_exp_1 (&tmp, 0, 0, 0);
+  cbuf = exp_end = exp_start + (tmp - start);
+
   /* Remove trailing whitespace from the expression before saving it.
      This makes the eventual display of the expression string a bit
      prettier.  */
@@ -10989,17 +11004,20 @@ watch_command_1 (char *arg, int accessfl
   else if (val != NULL)
     release_value (val);
 
-  tok = skip_spaces (arg);
-  end_tok = skip_to_space (tok);
+  tok = skip_spaces_const (cbuf);
+  end_tok = skip_to_space_const (tok);
 
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
     {
       struct expression *cond;
+      const char *tmp, *start;
 
       innermost_block = NULL;
       tok = cond_start = end_tok + 1;
-      cond = parse_exp_1 (&tok, 0, 0, 0);
+      tmp = start = tok;
+      cond = parse_exp_1 (&tmp, 0, 0, 0);
+      tok += tmp - start;
 
       /* The watchpoint expression may not be local, but the condition
 	 may still be.  E.g.: `watch global if local > 0'.  */
@@ -14036,7 +14054,7 @@ update_breakpoint_locations (struct brea
 	 old symtab.  */
       if (b->cond_string != NULL)
 	{
-	  char *s;
+	  const char *s;
 	  volatile struct gdb_exception e;
 
 	  s = b->cond_string;
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.178
diff -u -p -r1.178 eval.c
--- eval.c	14 Feb 2013 12:43:45 -0000	1.178
+++ eval.c	7 Mar 2013 22:59:29 -0000
@@ -76,7 +76,7 @@ evaluate_subexp (struct type *expect_typ
    and return the result as a number.  */
 
 CORE_ADDR
-parse_and_eval_address (char *exp)
+parse_and_eval_address (const char *exp)
 {
   struct expression *expr = parse_expression (exp);
   CORE_ADDR addr;
@@ -121,7 +121,7 @@ parse_and_eval (char *exp)
    EXPP is advanced to point to the comma.  */
 
 struct value *
-parse_to_comma_and_eval (char **expp)
+parse_to_comma_and_eval (const char **expp)
 {
   struct expression *expr = parse_exp_1 (expp, 0, (struct block *) 0, 1);
   struct value *val;
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.47
diff -u -p -r1.47 expression.h
--- expression.h	1 Jan 2013 06:32:42 -0000	1.47
+++ expression.h	7 Mar 2013 22:59:29 -0000
@@ -95,12 +95,12 @@ struct expression
 
 /* From parse.c */
 
-extern struct expression *parse_expression (char *);
+extern struct expression *parse_expression (const char *);
 
 extern struct type *parse_expression_for_completion (char *, char **,
 						     enum type_code *);
 
-extern struct expression *parse_exp_1 (char **, CORE_ADDR pc,
+extern struct expression *parse_exp_1 (const char **, CORE_ADDR pc,
 				       const struct block *, int);
 
 /* For use by parsers; set if we want to parse an expression and
Index: findcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/findcmd.c,v
retrieving revision 1.22
diff -u -p -r1.22 findcmd.c
--- findcmd.c	7 Mar 2013 21:57:29 -0000	1.22
+++ findcmd.c	7 Mar 2013 22:59:29 -0000
@@ -69,7 +69,7 @@ parse_find_args (char *args, ULONGEST *m
   ULONGEST pattern_len;
   CORE_ADDR start_addr;
   ULONGEST search_space_len;
-  char *s = args;
+  const char *s = args;
   struct cleanup *old_cleanups;
   struct value *v;
 
@@ -110,7 +110,7 @@ parse_find_args (char *args, ULONGEST *m
 	    }
 	}
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   /* Get the search range.  */
@@ -120,7 +120,7 @@ parse_find_args (char *args, ULONGEST *m
 
   if (*s == ',')
     ++s;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s == '+')
     {
@@ -171,7 +171,7 @@ parse_find_args (char *args, ULONGEST *m
       struct type *t;
       ULONGEST pattern_buf_size_need;
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
 
       v = parse_to_comma_and_eval (&s);
       t = value_type (v);
@@ -219,7 +219,7 @@ parse_find_args (char *args, ULONGEST *m
 
       if (*s == ',')
 	++s;
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   if (pattern_buf_end == pattern_buf)
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.180
diff -u -p -r1.180 linespec.c
--- linespec.c	6 Mar 2013 11:05:54 -0000	1.180
+++ linespec.c	7 Mar 2013 22:59:29 -0000
@@ -326,7 +326,7 @@ static void iterate_over_file_blocks (st
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (char **exp_ptr);
+static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -2181,7 +2181,8 @@ parse_linespec (linespec_parser *parser,
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
   if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
     {
-      char *expr, *copy;
+      char *expr;
+      const char *copy;
 
       /* User specified an expression, *EXPR.  */
       copy = expr = copy_token_string (token);
@@ -2565,7 +2566,7 @@ initialize_defaults (struct symtab **def
    advancing EXP_PTR past any parsed text.  */
 
 static CORE_ADDR
-linespec_expression_to_pc (char **exp_ptr)
+linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
     /* The error message doesn't really matter, because this case
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.135
diff -u -p -r1.135 parse.c
--- parse.c	13 Jan 2013 18:57:00 -0000	1.135
+++ parse.c	7 Mar 2013 22:59:30 -0000
@@ -1125,10 +1125,18 @@ prefixify_subexp (struct expression *ine
    If COMMA is nonzero, stop if a comma is reached.  */
 
 struct expression *
-parse_exp_1 (char **stringptr, CORE_ADDR pc, const struct block *block,
+parse_exp_1 (const char **stringptr, CORE_ADDR pc, const struct block *block,
 	     int comma)
 {
-  return parse_exp_in_context (stringptr, pc, block, comma, 0, NULL);
+  struct expression *expr;
+  char *const_hack = *stringptr ? xstrdup (*stringptr) : NULL;
+  char *orig = const_hack;
+  struct cleanup *back_to = make_cleanup (xfree, const_hack);
+
+  expr = parse_exp_in_context (&const_hack, pc, block, comma, 0, NULL);
+  (*stringptr) += const_hack - orig;
+  do_cleanups (back_to);
+  return expr;
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
@@ -1264,7 +1272,7 @@ parse_exp_in_context (char **stringptr, 
    to use up all of the contents of STRING.  */
 
 struct expression *
-parse_expression (char *string)
+parse_expression (const char *string)
 {
   struct expression *exp;
 
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.223
diff -u -p -r1.223 printcmd.c
--- printcmd.c	11 Feb 2013 22:44:23 -0000	1.223
+++ printcmd.c	7 Mar 2013 22:59:30 -0000
@@ -2215,10 +2215,10 @@ printf_pointer (struct ui_file *stream, 
 /* printf "printf format string" ARG to STREAM.  */
 
 static void
-ui_printf (char *arg, struct ui_file *stream)
+ui_printf (const char *arg, struct ui_file *stream)
 {
   struct format_piece *fpieces;
-  char *s = arg;
+  const char *s = arg;
   struct value **val_args;
   int allocated_args = 20;
   struct cleanup *old_cleanups;
@@ -2229,7 +2229,7 @@ ui_printf (char *arg, struct ui_file *st
   if (s == 0)
     error_no_arg (_("format-control string and values to print"));
 
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   /* A format string should follow, enveloped in double quotes.  */
   if (*s++ != '"')
@@ -2242,14 +2242,14 @@ ui_printf (char *arg, struct ui_file *st
   if (*s++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s != ',' && *s != 0)
     error (_("Invalid argument syntax"));
 
   if (*s == ',')
     s++;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   {
     int nargs = 0;
@@ -2267,7 +2267,7 @@ ui_printf (char *arg, struct ui_file *st
 
     while (*s != '\0')
       {
-	char *s1;
+	const char *s1;
 
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.283
diff -u -p -r1.283 tracepoint.c
--- tracepoint.c	7 Mar 2013 21:57:30 -0000	1.283
+++ tracepoint.c	7 Mar 2013 22:59:30 -0000
@@ -749,9 +749,13 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
-	      exp = parse_exp_1 (&p, loc->address,
+	      const char *q, *o;
+
+	      o = q = tmp_p;
+
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p += q - o;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      if (exp->elts[0].opcode == OP_VAR_VALUE)
@@ -801,10 +805,13 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
+	      const char *o, *q;
+
+	      o = q = tmp_p;
 	      /* Only expressions are allowed for this action.  */
-	      exp = parse_exp_1 (&p, loc->address,
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p += q - o;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      /* We have something to evaluate, make sure that the expr to
@@ -1465,9 +1472,12 @@ encode_actions_1 (struct command_line *a
 		  unsigned long addr;
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q, *o;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  o = q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp += q - o;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  switch (exp->elts[0].opcode)
@@ -1556,9 +1566,12 @@ encode_actions_1 (struct command_line *a
 		{
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q, *o;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  o = q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp += q - o;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  aexpr = gen_eval_for_expr (tloc->address, exp);
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.217
diff -u -p -r1.217 value.h
--- value.h	6 Feb 2013 19:40:04 -0000	1.217
+++ value.h	7 Mar 2013 22:59:30 -0000
@@ -726,11 +726,11 @@ extern struct value *evaluate_subexp_wit
 
 extern struct value *parse_and_eval (char *exp);
 
-extern struct value *parse_to_comma_and_eval (char **expp);
+extern struct value *parse_to_comma_and_eval (const char **expp);
 
 extern struct type *parse_and_eval_type (char *p, int length);
 
-extern CORE_ADDR parse_and_eval_address (char *exp);
+extern CORE_ADDR parse_and_eval_address (const char *exp);
 
 extern LONGEST parse_and_eval_long (char *exp);
 
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.205
diff -u -p -r1.205 varobj.c
--- varobj.c	7 Mar 2013 19:24:32 -0000	1.205
+++ varobj.c	7 Mar 2013 22:59:30 -0000
@@ -620,7 +620,7 @@ varobj_create (char *objname,
       struct frame_info *fi;
       struct frame_id old_id = null_frame_id;
       struct block *block;
-      char *p;
+      const char *p;
       enum varobj_languages lang;
       struct value *value = NULL;
       volatile struct gdb_exception except;
@@ -1469,7 +1469,7 @@ varobj_set_value (struct varobj *var, ch
   struct expression *exp;
   struct value *value = NULL; /* Initialize to keep gcc happy.  */
   int saved_input_radix = input_radix;
-  char *s = expression;
+  const char *s = expression;
   volatile struct gdb_exception except;
 
   gdb_assert (varobj_editable_p (var));
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.150
diff -u -p -r1.150 cli-cmds.c
--- cli/cli-cmds.c	7 Mar 2013 21:57:30 -0000	1.150
+++ cli/cli-cmds.c	7 Mar 2013 22:59:30 -0000
@@ -1113,20 +1113,22 @@ disassemble_command (char *arg, int from
   const char *name;
   CORE_ADDR pc;
   int flags;
+  const char *p;
 
+  p = arg;
   name = NULL;
   flags = 0;
 
-  if (arg && *arg == '/')
+  if (p && *p == '/')
     {
-      ++arg;
+      ++p;
 
-      if (*arg == '\0')
+      if (*p == '\0')
 	error (_("Missing modifier."));
 
-      while (*arg && ! isspace (*arg))
+      while (*p && ! isspace (*p))
 	{
-	  switch (*arg++)
+	  switch (*p++)
 	    {
 	    case 'm':
 	      flags |= DISASSEMBLY_SOURCE;
@@ -1139,20 +1141,20 @@ disassemble_command (char *arg, int from
 	    }
 	}
 
-      arg = skip_spaces (arg);
+      p = skip_spaces_const (p);
     }
 
-  if (! arg || ! *arg)
+  if (! p || ! *p)
     {
       flags |= DISASSEMBLY_OMIT_FNAME;
       disassemble_current_function (flags);
       return;
     }
 
-  pc = value_as_address (parse_to_comma_and_eval (&arg));
-  if (arg[0] == ',')
-    ++arg;
-  if (arg[0] == '\0')
+  pc = value_as_address (parse_to_comma_and_eval (&p));
+  if (p[0] == ',')
+    ++p;
+  if (p[0] == '\0')
     {
       /* One argument.  */
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
@@ -1172,13 +1174,13 @@ disassemble_command (char *arg, int from
       /* Two arguments.  */
       int incl_flag = 0;
       low = pc;
-      arg = skip_spaces (arg);
-      if (arg[0] == '+')
+      p = skip_spaces_const (p);
+      if (p[0] == '+')
 	{
-	  ++arg;
+	  ++p;
 	  incl_flag = 1;
 	}
-      high = parse_and_eval_address (arg);
+      high = parse_and_eval_address (p);
       if (incl_flag)
 	high += low;
     }
Index: cli/cli-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.c,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.c
--- cli/cli-utils.c	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.c	7 Mar 2013 22:59:30 -0000
@@ -237,8 +237,8 @@ skip_spaces_const (const char *chp)
 
 /* See documentation in cli-utils.h.  */
 
-char *
-skip_to_space (char *chp)
+const char *
+skip_to_space_const (const char *chp)
 {
   if (chp == NULL)
     return NULL;
Index: cli/cli-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.h,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.h
--- cli/cli-utils.h	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.h	7 Mar 2013 22:59:30 -0000
@@ -101,7 +101,9 @@ extern const char *skip_spaces_const (co
 /* Skip leading non-whitespace characters in INP, returning an updated
    pointer.  If INP is NULL, return NULL.  */
 
-extern char *skip_to_space (char *inp);
+#define skip_to_space(INP) ((char *) skip_to_space_const ((INP)))
+
+extern const char *skip_to_space_const (const char *inp);
 
 /* Reverse S to the last non-whitespace character without skipping past
    START.  */
Index: common/format.c
===================================================================
RCS file: /cvs/src/src/gdb/common/format.c,v
retrieving revision 1.3
diff -u -p -r1.3 format.c
--- common/format.c	8 Feb 2013 22:52:20 -0000	1.3
+++ common/format.c	7 Mar 2013 22:59:30 -0000
@@ -28,11 +28,12 @@
 #include "format.h"
 
 struct format_piece *
-parse_format_string (char **arg)
+parse_format_string (const char **arg)
 {
-  char *s, *f, *string;
-  char *prev_start;
-  char *percent_loc;
+  const char *s;
+  char *f, *string;
+  const char *prev_start;
+  const char *percent_loc;
   char *sub_start, *current_substring;
   struct format_piece *pieces;
   int next_frag;
Index: common/format.h
===================================================================
RCS file: /cvs/src/src/gdb/common/format.h,v
retrieving revision 1.2
diff -u -p -r1.2 format.h
--- common/format.h	1 Jan 2013 06:32:54 -0000	1.2
+++ common/format.h	7 Mar 2013 22:59:30 -0000
@@ -51,7 +51,7 @@ struct format_piece
 /* Return an array of printf fragments found at the given string, and
    rewrite ARG with a pointer to the end of the format string.  */
 
-extern struct format_piece *parse_format_string (char **arg);
+extern struct format_piece *parse_format_string (const char **arg);
 
 /* Given a pointer to an array of format pieces, free any memory that
    would have been allocated by parse_format_string.  */
Index: gdbserver/ax.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/ax.c,v
retrieving revision 1.6
diff -u -p -r1.6 ax.c
--- gdbserver/ax.c	18 Jan 2013 06:40:57 -0000	1.6
+++ gdbserver/ax.c	7 Mar 2013 22:59:30 -0000
@@ -798,10 +798,10 @@ compile_bytecodes (struct agent_expr *ae
    in.  */
 
 static void
-ax_printf (CORE_ADDR fn, CORE_ADDR chan, char *format,
+ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
 	   int nargs, ULONGEST *args)
 {
-  char *f = format;
+  const char *f = format;
   struct format_piece *fpieces;
   int i, fp;
   char *current_substring;

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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08  0:12 [RFA] "constify" parse_exp_1 Keith Seitz
@ 2013-03-08  0:22 ` Pedro Alves
  2013-03-08  1:02   ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2013-03-08  0:22 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On 03/08/2013 12:09 AM, Keith Seitz wrote:
> --- ada-lang.c	5 Mar 2013 21:15:34 -0000	1.394
> +++ ada-lang.c	7 Mar 2013 22:59:29 -0000
> @@ -4057,13 +4057,14 @@ ada_read_renaming_var_value (struct symb
>  			     struct block *block)
>  {
>    char *sym_name;
> +  const char *sname;
>    struct expression *expr;
>    struct value *value;
>    struct cleanup *old_chain = NULL;
>  
> -  sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
> +  sname = sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
>    old_chain = make_cleanup (xfree, sym_name);
> -  expr = parse_exp_1 (&sym_name, 0, block, 0);
> +  expr = parse_exp_1 (&sname, 0, block, 0);
>    make_cleanup (free_current_contents, &expr);
>    value = evaluate_expression (expr);
>  

I didn't get past the first hunk in the patch.  :-)

The xstrdup seemed to be there _because_ parse_exp_1
changed the input string.  It seems it can be removed now.
Your patch seems to make sym_name unused, even.  There may be
more instances of this in the patch.  /me off to bed.  :-)

-- 
Pedro Alves


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08  0:22 ` Pedro Alves
@ 2013-03-08  1:02   ` Keith Seitz
  2013-03-08 12:27     ` Pedro Alves
  2013-03-08 14:54     ` Tom Tromey
  0 siblings, 2 replies; 13+ messages in thread
From: Keith Seitz @ 2013-03-08  1:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

On 03/07/2013 04:22 PM, Pedro Alves wrote:
 >
> I didn't get past the first hunk in the patch.  :-)

Gah! That's what happens (sometimes) when you get too used to making 
mechanical changes. My apologies!

> Your patch seems to make sym_name unused, even.  There may be
> more instances of this in the patch.  /me off to bed.  :-)

I *think* that's the only one. The only other places where a 
less-than-mechanical change is made is watch_command_1 (which is one 
hairy beast), find_condition_and_thread (which is still pretty trivial), 
and the tracepoint.c changes (which are almost as much a hairy beast).

So, allow me try again. Thank you! [And sleep well; this will be in your 
INBOX in the morning. :-P]

Keith

ChangeLog
2013-03-07  Keith Seitz  <keiths@redhat.com>

     * ada-lang.c (ada_read_renaming_var_value): Pass const
     pointer to expression string to parse_exp_1.
     (craete_excep_cond_exprs): Likewise.
     * ax-gdb.c (agent_eval_command_one): Likewise.
     (maint_agent_printf_command): Likewise.
     Constify much of the string handling/parsing.
     * breakpoint.c (set_breakpoint_condition): Pass const
     pointer to expression string to parse_exp_1.
     (update_watchpoint): Likewise.
     (parse_cmd_to_aexpr): Constify string handling.
     Pass const pointer to parse_exp_1.
     (init_breakpoint_sal): Pass const pointer to parse_exp_1.
     (find_condition_and_thread): Likewise.
     Make TOK const.
     (watch_command_1): Constify string handling.
     (update_breakpoint_location): Pass const pointer to
     parse_exp_1.
     * eval.c (parse_and_eval_address): Make EXP const.
     (parse_to_comma_and_eval): Make EXPP const.
     * expression.h (parse_expression): Make argument const.
     (parse_exp_1): Make first argument const.
     * findcmd.c (parse_find_args): Treat ARGS as const.
     * linespec.c (parse_linespec): Pass const pointer to
     linespec_expression_to_pc.
     (linespec_expression_to_pc): Make EXP_PTR const.
     * parse.c (parse_exp_1): Make STRINGPTR const.
     Make a copy of the expression to pass to parse_exp_in_context until
     this whole interface can be constified.
     (parse_expression): Make STRING const.
     * printcmd.c (ui_printf): Treat ARG as const.
     Handle const strings.
     * tracepoint.c (validate_actionline): Pass const pointer to
     all calls to parse_exp_1.
     (encode_actions_1): Likewise.
     * value.h (parse_to_comma_and_eval): Make argument const.
     (parse_and_eval_address): Likewise.
     * varobj.c (varobj_create): Pass const pointer to parse_exp_1.
     (varobj_set_value): Likewise.
     * cli/cli-cmds.c (disassemble_command): Treat ARG as const and
     constify string handling.
     Pass const pointers to parse_and_eval_address and
     parse_to_comman_and_eval.
     * cli/cli-utils.c (skip_to_space): Rename to ...
     (skip_to_space_const): ... this. Handle const strings.
     * cli/cli-utils.h (skip_to_space): Turn into macro which invokes
     skip_to_space_const.
     (skip_to_space_const): Declare.
     * common/format.c (parse_format_string): Make ARG const.
     Handle const strings.
     * common/format.h (parse_format_string): Make ARG const.
     * gdbserver/ax.c (ax_printf): Make FORMAT const.

[-- Attachment #2: constify-parse_exp_1.patch --]
[-- Type: text/x-patch, Size: 27008 bytes --]

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.394
diff -u -p -r1.394 ada-lang.c
--- ada-lang.c	5 Mar 2013 21:15:34 -0000	1.394
+++ ada-lang.c	8 Mar 2013 00:52:43 -0000
@@ -4056,15 +4056,14 @@ static struct value *
 ada_read_renaming_var_value (struct symbol *renaming_sym,
 			     struct block *block)
 {
-  char *sym_name;
+  const char *sym_name;
   struct expression *expr;
   struct value *value;
   struct cleanup *old_chain = NULL;
 
-  sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
-  old_chain = make_cleanup (xfree, sym_name);
+  sym_name = SYMBOL_LINKAGE_NAME (renaming_sym);
   expr = parse_exp_1 (&sym_name, 0, block, 0);
-  make_cleanup (free_current_contents, &expr);
+  old_chain = make_cleanup (free_current_contents, &expr);
   value = evaluate_expression (expr);
 
   do_cleanups (old_chain);
@@ -11385,7 +11384,7 @@ create_excep_cond_exprs (struct ada_catc
       if (!bl->shlib_disabled)
 	{
 	  volatile struct gdb_exception e;
-	  char *s;
+	  const char *s;
 
 	  s = cond_string;
 	  TRY_CATCH (e, RETURN_MASK_ERROR)
Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.109
diff -u -p -r1.109 ax-gdb.c
--- ax-gdb.c	7 Mar 2013 00:48:25 -0000	1.109
+++ ax-gdb.c	8 Mar 2013 00:52:43 -0000
@@ -2608,6 +2608,7 @@ agent_eval_command_one (char *exp, int e
   struct cleanup *old_chain = 0;
   struct expression *expr;
   struct agent_expr *agent;
+  const char *arg;
 
   if (!eval)
     {
@@ -2616,14 +2617,15 @@ agent_eval_command_one (char *exp, int e
         exp = decode_agent_options (exp);
     }
 
-  if (!eval && strcmp (exp, "$_ret") == 0)
+  arg = exp;
+  if (!eval && strcmp (arg, "$_ret") == 0)
     {
       agent = gen_trace_for_return_address (pc, get_current_arch ());
       old_chain = make_cleanup_free_agent_expr (agent);
     }
   else
     {
-      expr = parse_exp_1 (&exp, pc, block_for_pc (pc), 0);
+      expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
       old_chain = make_cleanup (free_current_contents, &expr);
       if (eval)
 	agent = gen_eval_for_expr (pc, expr);
@@ -2716,8 +2718,8 @@ maint_agent_printf_command (char *exp, i
   struct expression *argvec[100];
   struct agent_expr *agent;
   struct frame_info *fi = get_current_frame ();	/* need current scope */
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
 
@@ -2733,7 +2735,7 @@ maint_agent_printf_command (char *exp, i
 
   cmdrest = exp;
 
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("Must start with a format string."));
@@ -2749,19 +2751,19 @@ maint_agent_printf_command (char *exp, i
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest != ',' && *cmdrest != 0)
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.744
diff -u -p -r1.744 breakpoint.c
--- breakpoint.c	7 Mar 2013 21:57:29 -0000	1.744
+++ breakpoint.c	8 Mar 2013 00:52:44 -0000
@@ -950,7 +950,7 @@ set_breakpoint_condition (struct breakpo
     }
   else
     {
-      char *arg = exp;
+      const char *arg = exp;
 
       /* I don't know if it matters whether this is the string the user
 	 typed in or the decompiled expression.  */
@@ -1759,7 +1759,7 @@ update_watchpoint (struct watchpoint *b,
 
   if (within_current_scope && reparse)
     {
-      char *s;
+      const char *s;
 
       if (b->exp)
 	{
@@ -2186,8 +2186,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   struct agent_expr *aexpr = NULL;
   struct cleanup *old_chain = NULL;
   volatile struct gdb_exception ex;
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
   struct gdbarch *gdbarch = get_current_arch ();
@@ -2199,7 +2199,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
 
   if (*cmdrest == ',')
     ++cmdrest;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("No format string following the location"));
@@ -2215,14 +2215,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (!(*cmdrest == ',' || *cmdrest == '\0'))
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   /* For each argument, make an expression.  */
 
@@ -2232,7 +2232,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
@@ -9103,7 +9103,8 @@ init_breakpoint_sal (struct breakpoint *
 
       if (b->cond_string)
 	{
-	  char *arg = b->cond_string;
+	  const char *arg = b->cond_string;
+
 	  loc->cond = parse_exp_1 (&arg, loc->address,
 				   block_for_pc (loc->address), 0);
 	  if (*arg)
@@ -9374,7 +9375,7 @@ invalid_thread_id_error (int id)
    If no thread is found, *THREAD is set to -1.  */
 
 static void
-find_condition_and_thread (char *tok, CORE_ADDR pc,
+find_condition_and_thread (const char *tok, CORE_ADDR pc,
 			   char **cond_string, int *thread, int *task,
 			   char **rest)
 {
@@ -9385,12 +9386,12 @@ find_condition_and_thread (char *tok, CO
 
   while (tok && *tok)
     {
-      char *end_tok;
+      const char *end_tok;
       int toklen;
-      char *cond_start = NULL;
-      char *cond_end = NULL;
+      const char *cond_start = NULL;
+      const char *cond_end = NULL;
 
-      tok = skip_spaces (tok);
+      tok = skip_spaces_const (tok);
 
       if ((*tok == '"' || *tok == ',') && rest)
 	{
@@ -9398,7 +9399,7 @@ find_condition_and_thread (char *tok, CO
 	  return;
 	}
 
-      end_tok = skip_to_space (tok);
+      end_tok = skip_to_space_const (tok);
 
       toklen = end_tok - tok;
 
@@ -9414,11 +9415,13 @@ find_condition_and_thread (char *tok, CO
 	}
       else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
 	{
-	  char *tmptok;
+	  char *endp;
+	  const char *tmptok;
 
 	  tok = end_tok + 1;
 	  tmptok = tok;
-	  *thread = strtol (tok, &tok, 0);
+	  *thread = strtol (tok, &endp, 0);
+	  tok = endp;
 	  if (tok == tmptok)
 	    error (_("Junk after thread keyword."));
 	  if (!valid_thread_id (*thread))
@@ -9426,11 +9429,13 @@ find_condition_and_thread (char *tok, CO
 	}
       else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
 	{
-	  char *tmptok;
+	  char *endp;
+	  const char *tmptok;
 
 	  tok = end_tok + 1;
 	  tmptok = tok;
-	  *task = strtol (tok, &tok, 0);
+	  *task = strtol (tok, &endp, 0);
+	  tok = endp;
 	  if (tok == tmptok)
 	    error (_("Junk after task keyword."));
 	  if (!valid_task_id (*task))
@@ -10845,12 +10850,12 @@ watch_command_1 (char *arg, int accessfl
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   struct frame_info *frame;
-  char *exp_start = NULL;
-  char *exp_end = NULL;
-  char *tok, *end_tok;
+  const char *exp_start = NULL;
+  const char *exp_end = NULL;
+  const char *tok, *end_tok;
   int toklen = -1;
-  char *cond_start = NULL;
-  char *cond_end = NULL;
+  const char *cond_start = NULL;
+  const char *cond_end = NULL;
   enum bptype bp_type;
   int thread = -1;
   int pc = 0;
@@ -10859,15 +10864,19 @@ watch_command_1 (char *arg, int accessfl
   int use_mask = 0;
   CORE_ADDR mask = 0;
   struct watchpoint *w;
+  const char *tmp, *start, *end_arg, *cbuf;
+  char *buf;
 
   /* Make sure that we actually have parameters to parse.  */
   if (arg != NULL && arg[0] != '\0')
     {
-      char *value_start;
+      const char *value_start;
+
+      end_arg = arg + strlen (arg);
 
       /* Look for "parameter value" pairs at the end
 	 of the arguments string.  */
-      for (tok = arg + strlen (arg) - 1; tok > arg; tok--)
+      for (tok = end_arg - 1; tok > arg; tok--)
 	{
 	  /* Skip whitespace at the end of the argument list.  */
 	  while (tok > arg && (*tok == ' ' || *tok == '\t'))
@@ -10937,15 +10946,22 @@ watch_command_1 (char *arg, int accessfl
 
 	  /* Truncate the string and get rid of the "parameter value" pair before
 	     the arguments string is parsed by the parse_exp_1 function.  */
-	  *tok = '\0';
+	  end_arg = tok;
 	}
     }
+  else
+    end_arg = arg;
 
   /* Parse the rest of the arguments.  */
   innermost_block = NULL;
-  exp_start = arg;
-  exp = parse_exp_1 (&arg, 0, 0, 0);
-  exp_end = arg;
+  buf = alloca (end_arg - arg + 1);
+  strncpy (buf, arg, end_arg - arg);
+  buf[end_arg - arg] = '\0';
+  exp_start = buf;
+  tmp = start = buf;
+  exp = parse_exp_1 (&tmp, 0, 0, 0);
+  cbuf = exp_end = exp_start + (tmp - start);
+
   /* Remove trailing whitespace from the expression before saving it.
      This makes the eventual display of the expression string a bit
      prettier.  */
@@ -10989,17 +11005,20 @@ watch_command_1 (char *arg, int accessfl
   else if (val != NULL)
     release_value (val);
 
-  tok = skip_spaces (arg);
-  end_tok = skip_to_space (tok);
+  tok = skip_spaces_const (cbuf);
+  end_tok = skip_to_space_const (tok);
 
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
     {
       struct expression *cond;
+      const char *tmp, *start;
 
       innermost_block = NULL;
       tok = cond_start = end_tok + 1;
-      cond = parse_exp_1 (&tok, 0, 0, 0);
+      tmp = start = tok;
+      cond = parse_exp_1 (&tmp, 0, 0, 0);
+      tok += tmp - start;
 
       /* The watchpoint expression may not be local, but the condition
 	 may still be.  E.g.: `watch global if local > 0'.  */
@@ -14036,7 +14055,7 @@ update_breakpoint_locations (struct brea
 	 old symtab.  */
       if (b->cond_string != NULL)
 	{
-	  char *s;
+	  const char *s;
 	  volatile struct gdb_exception e;
 
 	  s = b->cond_string;
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.178
diff -u -p -r1.178 eval.c
--- eval.c	14 Feb 2013 12:43:45 -0000	1.178
+++ eval.c	8 Mar 2013 00:52:44 -0000
@@ -76,7 +76,7 @@ evaluate_subexp (struct type *expect_typ
    and return the result as a number.  */
 
 CORE_ADDR
-parse_and_eval_address (char *exp)
+parse_and_eval_address (const char *exp)
 {
   struct expression *expr = parse_expression (exp);
   CORE_ADDR addr;
@@ -121,7 +121,7 @@ parse_and_eval (char *exp)
    EXPP is advanced to point to the comma.  */
 
 struct value *
-parse_to_comma_and_eval (char **expp)
+parse_to_comma_and_eval (const char **expp)
 {
   struct expression *expr = parse_exp_1 (expp, 0, (struct block *) 0, 1);
   struct value *val;
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.47
diff -u -p -r1.47 expression.h
--- expression.h	1 Jan 2013 06:32:42 -0000	1.47
+++ expression.h	8 Mar 2013 00:52:44 -0000
@@ -95,12 +95,12 @@ struct expression
 
 /* From parse.c */
 
-extern struct expression *parse_expression (char *);
+extern struct expression *parse_expression (const char *);
 
 extern struct type *parse_expression_for_completion (char *, char **,
 						     enum type_code *);
 
-extern struct expression *parse_exp_1 (char **, CORE_ADDR pc,
+extern struct expression *parse_exp_1 (const char **, CORE_ADDR pc,
 				       const struct block *, int);
 
 /* For use by parsers; set if we want to parse an expression and
Index: findcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/findcmd.c,v
retrieving revision 1.22
diff -u -p -r1.22 findcmd.c
--- findcmd.c	7 Mar 2013 21:57:29 -0000	1.22
+++ findcmd.c	8 Mar 2013 00:52:44 -0000
@@ -69,7 +69,7 @@ parse_find_args (char *args, ULONGEST *m
   ULONGEST pattern_len;
   CORE_ADDR start_addr;
   ULONGEST search_space_len;
-  char *s = args;
+  const char *s = args;
   struct cleanup *old_cleanups;
   struct value *v;
 
@@ -110,7 +110,7 @@ parse_find_args (char *args, ULONGEST *m
 	    }
 	}
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   /* Get the search range.  */
@@ -120,7 +120,7 @@ parse_find_args (char *args, ULONGEST *m
 
   if (*s == ',')
     ++s;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s == '+')
     {
@@ -171,7 +171,7 @@ parse_find_args (char *args, ULONGEST *m
       struct type *t;
       ULONGEST pattern_buf_size_need;
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
 
       v = parse_to_comma_and_eval (&s);
       t = value_type (v);
@@ -219,7 +219,7 @@ parse_find_args (char *args, ULONGEST *m
 
       if (*s == ',')
 	++s;
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   if (pattern_buf_end == pattern_buf)
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.180
diff -u -p -r1.180 linespec.c
--- linespec.c	6 Mar 2013 11:05:54 -0000	1.180
+++ linespec.c	8 Mar 2013 00:52:44 -0000
@@ -326,7 +326,7 @@ static void iterate_over_file_blocks (st
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (char **exp_ptr);
+static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -2181,7 +2181,8 @@ parse_linespec (linespec_parser *parser,
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
   if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
     {
-      char *expr, *copy;
+      char *expr;
+      const char *copy;
 
       /* User specified an expression, *EXPR.  */
       copy = expr = copy_token_string (token);
@@ -2565,7 +2566,7 @@ initialize_defaults (struct symtab **def
    advancing EXP_PTR past any parsed text.  */
 
 static CORE_ADDR
-linespec_expression_to_pc (char **exp_ptr)
+linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
     /* The error message doesn't really matter, because this case
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.135
diff -u -p -r1.135 parse.c
--- parse.c	13 Jan 2013 18:57:00 -0000	1.135
+++ parse.c	8 Mar 2013 00:52:44 -0000
@@ -1125,10 +1125,18 @@ prefixify_subexp (struct expression *ine
    If COMMA is nonzero, stop if a comma is reached.  */
 
 struct expression *
-parse_exp_1 (char **stringptr, CORE_ADDR pc, const struct block *block,
+parse_exp_1 (const char **stringptr, CORE_ADDR pc, const struct block *block,
 	     int comma)
 {
-  return parse_exp_in_context (stringptr, pc, block, comma, 0, NULL);
+  struct expression *expr;
+  char *const_hack = *stringptr ? xstrdup (*stringptr) : NULL;
+  char *orig = const_hack;
+  struct cleanup *back_to = make_cleanup (xfree, const_hack);
+
+  expr = parse_exp_in_context (&const_hack, pc, block, comma, 0, NULL);
+  (*stringptr) += const_hack - orig;
+  do_cleanups (back_to);
+  return expr;
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
@@ -1264,7 +1272,7 @@ parse_exp_in_context (char **stringptr, 
    to use up all of the contents of STRING.  */
 
 struct expression *
-parse_expression (char *string)
+parse_expression (const char *string)
 {
   struct expression *exp;
 
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.223
diff -u -p -r1.223 printcmd.c
--- printcmd.c	11 Feb 2013 22:44:23 -0000	1.223
+++ printcmd.c	8 Mar 2013 00:52:44 -0000
@@ -2215,10 +2215,10 @@ printf_pointer (struct ui_file *stream, 
 /* printf "printf format string" ARG to STREAM.  */
 
 static void
-ui_printf (char *arg, struct ui_file *stream)
+ui_printf (const char *arg, struct ui_file *stream)
 {
   struct format_piece *fpieces;
-  char *s = arg;
+  const char *s = arg;
   struct value **val_args;
   int allocated_args = 20;
   struct cleanup *old_cleanups;
@@ -2229,7 +2229,7 @@ ui_printf (char *arg, struct ui_file *st
   if (s == 0)
     error_no_arg (_("format-control string and values to print"));
 
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   /* A format string should follow, enveloped in double quotes.  */
   if (*s++ != '"')
@@ -2242,14 +2242,14 @@ ui_printf (char *arg, struct ui_file *st
   if (*s++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s != ',' && *s != 0)
     error (_("Invalid argument syntax"));
 
   if (*s == ',')
     s++;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   {
     int nargs = 0;
@@ -2267,7 +2267,7 @@ ui_printf (char *arg, struct ui_file *st
 
     while (*s != '\0')
       {
-	char *s1;
+	const char *s1;
 
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.283
diff -u -p -r1.283 tracepoint.c
--- tracepoint.c	7 Mar 2013 21:57:30 -0000	1.283
+++ tracepoint.c	8 Mar 2013 00:52:44 -0000
@@ -749,9 +749,13 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
-	      exp = parse_exp_1 (&p, loc->address,
+	      const char *q, *o;
+
+	      o = q = tmp_p;
+
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p += q - o;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      if (exp->elts[0].opcode == OP_VAR_VALUE)
@@ -801,10 +805,13 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
+	      const char *o, *q;
+
+	      o = q = tmp_p;
 	      /* Only expressions are allowed for this action.  */
-	      exp = parse_exp_1 (&p, loc->address,
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p += q - o;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      /* We have something to evaluate, make sure that the expr to
@@ -1465,9 +1472,12 @@ encode_actions_1 (struct command_line *a
 		  unsigned long addr;
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q, *o;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  o = q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp += q - o;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  switch (exp->elts[0].opcode)
@@ -1556,9 +1566,12 @@ encode_actions_1 (struct command_line *a
 		{
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q, *o;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  o = q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp += q - o;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  aexpr = gen_eval_for_expr (tloc->address, exp);
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.217
diff -u -p -r1.217 value.h
--- value.h	6 Feb 2013 19:40:04 -0000	1.217
+++ value.h	8 Mar 2013 00:52:44 -0000
@@ -726,11 +726,11 @@ extern struct value *evaluate_subexp_wit
 
 extern struct value *parse_and_eval (char *exp);
 
-extern struct value *parse_to_comma_and_eval (char **expp);
+extern struct value *parse_to_comma_and_eval (const char **expp);
 
 extern struct type *parse_and_eval_type (char *p, int length);
 
-extern CORE_ADDR parse_and_eval_address (char *exp);
+extern CORE_ADDR parse_and_eval_address (const char *exp);
 
 extern LONGEST parse_and_eval_long (char *exp);
 
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.205
diff -u -p -r1.205 varobj.c
--- varobj.c	7 Mar 2013 19:24:32 -0000	1.205
+++ varobj.c	8 Mar 2013 00:52:44 -0000
@@ -620,7 +620,7 @@ varobj_create (char *objname,
       struct frame_info *fi;
       struct frame_id old_id = null_frame_id;
       struct block *block;
-      char *p;
+      const char *p;
       enum varobj_languages lang;
       struct value *value = NULL;
       volatile struct gdb_exception except;
@@ -1469,7 +1469,7 @@ varobj_set_value (struct varobj *var, ch
   struct expression *exp;
   struct value *value = NULL; /* Initialize to keep gcc happy.  */
   int saved_input_radix = input_radix;
-  char *s = expression;
+  const char *s = expression;
   volatile struct gdb_exception except;
 
   gdb_assert (varobj_editable_p (var));
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.150
diff -u -p -r1.150 cli-cmds.c
--- cli/cli-cmds.c	7 Mar 2013 21:57:30 -0000	1.150
+++ cli/cli-cmds.c	8 Mar 2013 00:52:44 -0000
@@ -1113,20 +1113,22 @@ disassemble_command (char *arg, int from
   const char *name;
   CORE_ADDR pc;
   int flags;
+  const char *p;
 
+  p = arg;
   name = NULL;
   flags = 0;
 
-  if (arg && *arg == '/')
+  if (p && *p == '/')
     {
-      ++arg;
+      ++p;
 
-      if (*arg == '\0')
+      if (*p == '\0')
 	error (_("Missing modifier."));
 
-      while (*arg && ! isspace (*arg))
+      while (*p && ! isspace (*p))
 	{
-	  switch (*arg++)
+	  switch (*p++)
 	    {
 	    case 'm':
 	      flags |= DISASSEMBLY_SOURCE;
@@ -1139,20 +1141,20 @@ disassemble_command (char *arg, int from
 	    }
 	}
 
-      arg = skip_spaces (arg);
+      p = skip_spaces_const (p);
     }
 
-  if (! arg || ! *arg)
+  if (! p || ! *p)
     {
       flags |= DISASSEMBLY_OMIT_FNAME;
       disassemble_current_function (flags);
       return;
     }
 
-  pc = value_as_address (parse_to_comma_and_eval (&arg));
-  if (arg[0] == ',')
-    ++arg;
-  if (arg[0] == '\0')
+  pc = value_as_address (parse_to_comma_and_eval (&p));
+  if (p[0] == ',')
+    ++p;
+  if (p[0] == '\0')
     {
       /* One argument.  */
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
@@ -1172,13 +1174,13 @@ disassemble_command (char *arg, int from
       /* Two arguments.  */
       int incl_flag = 0;
       low = pc;
-      arg = skip_spaces (arg);
-      if (arg[0] == '+')
+      p = skip_spaces_const (p);
+      if (p[0] == '+')
 	{
-	  ++arg;
+	  ++p;
 	  incl_flag = 1;
 	}
-      high = parse_and_eval_address (arg);
+      high = parse_and_eval_address (p);
       if (incl_flag)
 	high += low;
     }
Index: cli/cli-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.c,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.c
--- cli/cli-utils.c	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.c	8 Mar 2013 00:52:44 -0000
@@ -237,8 +237,8 @@ skip_spaces_const (const char *chp)
 
 /* See documentation in cli-utils.h.  */
 
-char *
-skip_to_space (char *chp)
+const char *
+skip_to_space_const (const char *chp)
 {
   if (chp == NULL)
     return NULL;
Index: cli/cli-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.h,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.h
--- cli/cli-utils.h	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.h	8 Mar 2013 00:52:44 -0000
@@ -101,7 +101,9 @@ extern const char *skip_spaces_const (co
 /* Skip leading non-whitespace characters in INP, returning an updated
    pointer.  If INP is NULL, return NULL.  */
 
-extern char *skip_to_space (char *inp);
+#define skip_to_space(INP) ((char *) skip_to_space_const ((INP)))
+
+extern const char *skip_to_space_const (const char *inp);
 
 /* Reverse S to the last non-whitespace character without skipping past
    START.  */
Index: common/format.c
===================================================================
RCS file: /cvs/src/src/gdb/common/format.c,v
retrieving revision 1.3
diff -u -p -r1.3 format.c
--- common/format.c	8 Feb 2013 22:52:20 -0000	1.3
+++ common/format.c	8 Mar 2013 00:52:44 -0000
@@ -28,11 +28,12 @@
 #include "format.h"
 
 struct format_piece *
-parse_format_string (char **arg)
+parse_format_string (const char **arg)
 {
-  char *s, *f, *string;
-  char *prev_start;
-  char *percent_loc;
+  const char *s;
+  char *f, *string;
+  const char *prev_start;
+  const char *percent_loc;
   char *sub_start, *current_substring;
   struct format_piece *pieces;
   int next_frag;
Index: common/format.h
===================================================================
RCS file: /cvs/src/src/gdb/common/format.h,v
retrieving revision 1.2
diff -u -p -r1.2 format.h
--- common/format.h	1 Jan 2013 06:32:54 -0000	1.2
+++ common/format.h	8 Mar 2013 00:52:44 -0000
@@ -51,7 +51,7 @@ struct format_piece
 /* Return an array of printf fragments found at the given string, and
    rewrite ARG with a pointer to the end of the format string.  */
 
-extern struct format_piece *parse_format_string (char **arg);
+extern struct format_piece *parse_format_string (const char **arg);
 
 /* Given a pointer to an array of format pieces, free any memory that
    would have been allocated by parse_format_string.  */
Index: gdbserver/ax.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/ax.c,v
retrieving revision 1.6
diff -u -p -r1.6 ax.c
--- gdbserver/ax.c	18 Jan 2013 06:40:57 -0000	1.6
+++ gdbserver/ax.c	8 Mar 2013 00:52:44 -0000
@@ -798,10 +798,10 @@ compile_bytecodes (struct agent_expr *ae
    in.  */
 
 static void
-ax_printf (CORE_ADDR fn, CORE_ADDR chan, char *format,
+ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
 	   int nargs, ULONGEST *args)
 {
-  char *f = format;
+  const char *f = format;
   struct format_piece *fpieces;
   int i, fp;
   char *current_substring;

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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08  1:02   ` Keith Seitz
@ 2013-03-08 12:27     ` Pedro Alves
  2013-03-08 13:55       ` Pedro Alves
  2013-03-08 14:54     ` Tom Tromey
  1 sibling, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2013-03-08 12:27 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On 03/08/2013 01:01 AM, Keith Seitz wrote:
>       else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
>  	{
> -	  char *tmptok;
> +	  char *endp;
> +	  const char *tmptok;
>  
>  	  tok = end_tok + 1;
>  	  tmptok = tok;
> -	  *thread = strtol (tok, &tok, 0);
> +	  *thread = strtol (tok, &endp, 0);
> +	  tok = endp;
>  	  if (tok == tmptok)
>  	    error (_("Junk after thread keyword."));

I don't think we don't need two temporaries.  The issue is
that the code passed "tok" for both input and output.

Seems like the simpler:

       else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
  	{
 	  char *tmptok;
  
  	  tok = end_tok + 1;
-  	  tmptok = tok;
- 	  *thread = strtol (tok, &tok, 0);
+ 	  *thread = strtol (tok, &tmptok, 0);
  	  if (tok == tmptok)
  	    error (_("Junk after thread keyword."));
+ 	  tok = tmptok;
         ...

should work.  See patchlet below (untested).
Fine with me to s/endp/tmptok/ if you want.

Same for "task".

> @@ -10845,12 +10850,12 @@ watch_command_1 (char *arg, int accessfl
>    const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
>    struct value *val, *mark, *result;
>    struct frame_info *frame;
> -  char *exp_start = NULL;
> -  char *exp_end = NULL;
> -  char *tok, *end_tok;
> +  const char *exp_start = NULL;
> +  const char *exp_end = NULL;
> +  const char *tok, *end_tok;
>    int toklen = -1;
> -  char *cond_start = NULL;
> -  char *cond_end = NULL;
> +  const char *cond_start = NULL;
> +  const char *cond_end = NULL;
>    enum bptype bp_type;
>    int thread = -1;
>    int pc = 0;
> @@ -10859,15 +10864,19 @@ watch_command_1 (char *arg, int accessfl
>    int use_mask = 0;
>    CORE_ADDR mask = 0;
>    struct watchpoint *w;
> +  const char *tmp, *start, *end_arg, *cbuf;
> +  char *buf;
>  
>    /* Make sure that we actually have parameters to parse.  */
>    if (arg != NULL && arg[0] != '\0')
>      {
> -      char *value_start;
> +      const char *value_start;
> +
> +      end_arg = arg + strlen (arg);
>  
>        /* Look for "parameter value" pairs at the end
>  	 of the arguments string.  */
> -      for (tok = arg + strlen (arg) - 1; tok > arg; tok--)
> +      for (tok = end_arg - 1; tok > arg; tok--)
>  	{
>  	  /* Skip whitespace at the end of the argument list.  */
>  	  while (tok > arg && (*tok == ' ' || *tok == '\t'))
> @@ -10937,15 +10946,22 @@ watch_command_1 (char *arg, int accessfl
>  
>  	  /* Truncate the string and get rid of the "parameter value" pair before
>  	     the arguments string is parsed by the parse_exp_1 function.  */
> -	  *tok = '\0';
> +	  end_arg = tok;
>  	}
>      }
> +  else
> +    end_arg = arg;
>  
>    /* Parse the rest of the arguments.  */
>    innermost_block = NULL;
> -  exp_start = arg;
> -  exp = parse_exp_1 (&arg, 0, 0, 0);
> -  exp_end = arg;
> +  buf = alloca (end_arg - arg + 1);
> +  strncpy (buf, arg, end_arg - arg);
> +  buf[end_arg - arg] = '\0';
> +  exp_start = buf;
> +  tmp = start = buf;
> +  exp = parse_exp_1 (&tmp, 0, 0, 0);
> +  cbuf = exp_end = exp_start + (tmp - start);
> +

I find this hard to grok.  :-/  "tmp, buf, cbuf, I'm lost, argh."  
I don't think we actually need all that hair.  Using
savestring+cleanup makes this much clearer to me (see below).



Looking at this wondered something (not for this patch):

  /* Remove trailing whitespace from the expression before saving it.
     This makes the eventual display of the expression string a bit
     prettier.  */
  while (exp_end > exp_start && (exp_end[-1] == ' ' || exp_end[-1] == '\t'))
    --exp_end;

I thought parse_exp_1 stopped before any trailing whitespace?

/* Read an expression from the string *STRINGPTR points to,
   parse it, and return a pointer to a struct expression that we malloc.
   Use block BLOCK as the lexical context for variable names;
   if BLOCK is zero, use the block of the selected stack frame.
   Meanwhile, advance *STRINGPTR to point after the expression,
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   at the first nonwhite character that is not part of the expression
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   (possibly a null character).

   If COMMA is nonzero, stop if a comma is reached.  */

struct expression *
parse_exp_1 (const char **stringptr, CORE_ADDR pc, const struct block *block,
	     int comma)



>    if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
>      {
>        struct expression *cond;
> +      const char *tmp, *start;
>  
>        innermost_block = NULL;
>        tok = cond_start = end_tok + 1;
> -      cond = parse_exp_1 (&tok, 0, 0, 0);
> +      tmp = start = tok;
> +      cond = parse_exp_1 (&tmp, 0, 0, 0);
> +      tok += tmp - start;

This hunk looks completely unnecessary.

> --- tracepoint.c	7 Mar 2013 21:57:30 -0000	1.283
> +++ tracepoint.c	8 Mar 2013 00:52:44 -0000
> @@ -749,9 +749,13 @@ validate_actionline (char **line, struct
>  	  tmp_p = p;
>  	  for (loc = t->base.loc; loc; loc = loc->next)
>  	    {
> -	      p = tmp_p;
> -	      exp = parse_exp_1 (&p, loc->address,
> +	      const char *q, *o;
> +
> +	      o = q = tmp_p;
> +
> +	      exp = parse_exp_1 (&q, loc->address,
>  				 block_for_pc (loc->address), 1);
> +	      p += q - o;
>  	      old_chain = make_cleanup (free_current_contents, &exp);
>  
>  	      if (exp->elts[0].opcode == OP_VAR_VALUE)

Argh all that extra pointer arithmetic hurts my eyes.

Do we really need it?  Does this:

-	      exp = parse_exp_1 (&p, loc->address,
+	      exp = parse_exp_1 ((const char **) &p, loc->address,

break strict aliasing rules?  If not, that's all it takes, and we can
later on work on progressively removing these casts outwards.  If it
does, then I'd much rather see all that pointer manipulation that
appears many times in the patch wrapped in a helper function
that looks like parse_exp_1 but still takes a char** ...

-- 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index fbb6c62..2afdd60 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9416,14 +9416,12 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
       else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
 	{
 	  char *endp;
-	  const char *tmptok;
 
 	  tok = end_tok + 1;
-	  tmptok = tok;
 	  *thread = strtol (tok, &endp, 0);
-	  tok = endp;
-	  if (tok == tmptok)
+	  if (endp == tok)
 	    error (_("Junk after thread keyword."));
+	  tok = endp;
 	  if (!valid_thread_id (*thread))
 	    invalid_thread_id_error (*thread);
 	}
@@ -10845,13 +10843,16 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
 		 int just_location, int internal)
 {
   volatile struct gdb_exception e;
+  struct cleanup *old_chain;
   struct breakpoint *b, *scope_breakpoint = NULL;
   struct expression *exp;
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   struct frame_info *frame;
+  const char *end_arg;
   const char *exp_start = NULL;
   const char *exp_end = NULL;
+  const char *exp_end_spaces;
   const char *tok, *end_tok;
   int toklen = -1;
   const char *cond_start = NULL;
@@ -10864,8 +10865,6 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
   int use_mask = 0;
   CORE_ADDR mask = 0;
   struct watchpoint *w;
-  const char *tmp, *start, *end_arg, *cbuf;
-  char *buf;
 
   /* Make sure that we actually have parameters to parse.  */
   if (arg != NULL && arg[0] != '\0')
@@ -10954,13 +10953,13 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
 
   /* Parse the rest of the arguments.  */
   innermost_block = NULL;
-  buf = alloca (end_arg - arg + 1);
-  strncpy (buf, arg, end_arg - arg);
-  buf[end_arg - arg] = '\0';
-  exp_start = buf;
-  tmp = start = buf;
-  exp = parse_exp_1 (&tmp, 0, 0, 0);
-  cbuf = exp_end = exp_start + (tmp - start);
+
+  exp_start = savestring (arg, end_arg - arg);
+  old_chain = make_cleanup (xfree, (char *) exp_start);
+
+  exp_end = exp_start;
+  exp = parse_exp_1 (&exp_end, 0, 0, 0);
+  exp_end_spaces = exp_end;
 
   /* Remove trailing whitespace from the expression before saving it.
      This makes the eventual display of the expression string a bit
@@ -11005,20 +11004,17 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
   else if (val != NULL)
     release_value (val);
 
-  tok = skip_spaces_const (cbuf);
+  tok = skip_spaces_const (exp_end_spaces);
   end_tok = skip_to_space_const (tok);
 
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
     {
       struct expression *cond;
-      const char *tmp, *start;
 
       innermost_block = NULL;
       tok = cond_start = end_tok + 1;
-      tmp = start = tok;
-      cond = parse_exp_1 (&tmp, 0, 0, 0);
-      tok += tmp - start;
+      cond = parse_exp_1 (&tok, 0, 0, 0);
 
       /* The watchpoint expression may not be local, but the condition
 	 may still be.  E.g.: `watch global if local > 0'.  */
@@ -11161,6 +11157,8 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
     }
 
   install_breakpoint (internal, b, 1);
+
+  do_cleanups (old_chain);
 }
 
 /* Return count of debug registers needed to watch the given expression.


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08 12:27     ` Pedro Alves
@ 2013-03-08 13:55       ` Pedro Alves
  2013-03-09  1:00         ` Pedro Alves
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2013-03-08 13:55 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On 03/08/2013 12:27 PM, Pedro Alves wrote:
>> > @@ -749,9 +749,13 @@ validate_actionline (char **line, struct
>> >  	  tmp_p = p;
>> >  	  for (loc = t->base.loc; loc; loc = loc->next)
>> >  	    {
>> > -	      p = tmp_p;
>> > -	      exp = parse_exp_1 (&p, loc->address,
>> > +	      const char *q, *o;
>> > +
>> > +	      o = q = tmp_p;
>> > +
>> > +	      exp = parse_exp_1 (&q, loc->address,
>> >  				 block_for_pc (loc->address), 1);
>> > +	      p += q - o;
>> >  	      old_chain = make_cleanup (free_current_contents, &exp);
>> >  
>> >  	      if (exp->elts[0].opcode == OP_VAR_VALUE)
> Argh all that extra pointer arithmetic hurts my eyes.
> 
> Do we really need it?  

BTW, over lunch I just had an epiphany.  :-)
This is perfectly fine:

 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
+	      const char *q;
+
+	      q = tmp_p;
-	      exp = parse_exp_1 (&p, loc->address,
+	      exp = parse_exp_1 (&q, loc->address,
				 block_for_pc (loc->address), 1);
+	      p = (char *) q;

It's perfectly valid, as we know Q on output must point within
the object/string TMP_P pointed at on entry.
This reads much more intuitively to me, no funny arithmetic, and
gets rid of the aliasing issue with the other suggestion, and
no new function necessary.

-- 
Pedro Alves


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08  1:02   ` Keith Seitz
  2013-03-08 12:27     ` Pedro Alves
@ 2013-03-08 14:54     ` Tom Tromey
  2013-03-11 16:50       ` Keith Seitz
  1 sibling, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2013-03-08 14:54 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Pedro Alves, gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Pedro> Your patch seems to make sym_name unused, even.  There may be
Pedro> more instances of this in the patch.  /me off to bed.  :-)

Keith> I *think* that's the only one.

There is a deletable strdup in gdbpy_parse_and_eval.
I don't know if there are others in the tree.
I think you need to look at the indirect callers of parse_exp_1.

Tom


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08 13:55       ` Pedro Alves
@ 2013-03-09  1:00         ` Pedro Alves
  2013-03-09  1:04           ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2013-03-09  1:00 UTC (permalink / raw)
  Cc: Keith Seitz, GDB Patches

On 03/08/2013 12:27 PM, Pedro Alves wrote:
> Do we really need it?  Does this:
>
> -	      exp = parse_exp_1 (&p, loc->address,
> +	      exp = parse_exp_1 ((const char **) &p, loc->address,
>
> break strict aliasing rules?

Joseph Meyers confirmed this is indeed not valid in C.

> BTW, over lunch I just had an epiphany.  :-)
> This is perfectly fine:
> 
>  	  for (loc = t->base.loc; loc; loc = loc->next)
>  	    {
> -	      p = tmp_p;
> +	      const char *q;
> +
> +	      q = tmp_p;
> -	      exp = parse_exp_1 (&p, loc->address,
> +	      exp = parse_exp_1 (&q, loc->address,
> 				 block_for_pc (loc->address), 1);
> +	      p = (char *) q;
> 
> It's perfectly valid, as we know Q on output must point within
> the object/string TMP_P pointed at on entry.
> This reads much more intuitively to me, no funny arithmetic, and
> gets rid of the aliasing issue with the other suggestion, and
> no new function necessary.

And confirmed this approach is valid.

-- 
Pedro Alves


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-09  1:00         ` Pedro Alves
@ 2013-03-09  1:04           ` Keith Seitz
  2013-03-11 22:09             ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2013-03-09  1:04 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches

On 03/08/2013 05:00 PM, Pedro Alves wrote:
>> This is perfectly fine:
>>
>>   	  for (loc = t->base.loc; loc; loc = loc->next)
>>   	    {
>> -	      p = tmp_p;
>> +	      const char *q;
>> +
>> +	      q = tmp_p;
>> -	      exp = parse_exp_1 (&p, loc->address,
>> +	      exp = parse_exp_1 (&q, loc->address,
>> 				 block_for_pc (loc->address), 1);
>> +	      p = (char *) q;
>>
>> It's perfectly valid, as we know Q on output must point within
>> the object/string TMP_P pointed at on entry.
>> This reads much more intuitively to me, no funny arithmetic, and
>> gets rid of the aliasing issue with the other suggestion, and
>> no new function necessary.
>
> And confirmed this approach is valid.

I have a revision, but after this morning's fiasco, I'm a bit leery of 
submitting it until substantial further review. My focus is waxing 
quickly right now.

I've also got amendments to address Tom's comments.

Thanks,
Keith


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-08 14:54     ` Tom Tromey
@ 2013-03-11 16:50       ` Keith Seitz
  2013-03-11 18:30         ` Tom Tromey
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2013-03-11 16:50 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, gdb-patches@sourceware.org ml

On 03/08/2013 06:53 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
> There is a deletable strdup in gdbpy_parse_and_eval.
> I don't know if there are others in the tree.
> I think you need to look at the indirect callers of parse_exp_1.

Ok, that strdup, while not necessary, is not yet ready to be removed 
until parse_and_eval is const, which I did not do. I thought that might 
be a much more involved patch, and planned to do that separately.

It turns out to be pretty simple, actually, so I'll include that in my 
next revision (see my reply to Pedro).

Analyzing the call graph (by hand!?), I found one other place that 
allocates a copy: mi_interpreter_exec. AFAICT, it is not needed. I will 
submit a patch for that separately.

I'm including a list of the callgraphs I investigated. Each '-' refers 
to a caller, e.g.:

- a
-- b
-- c
--- d

d calls c which calls a; b calls a. Anything marked with "X"s indicates 
that an alloc appears to have been made explicitly to workaround 
const-correctness. There are only two that I found (gdbpy_parse_and_eval 
and mi_interpreter_exec).

     parse_exp_1:
     - parse_expression (parse.c)
     -- execute_control_command (cli-script.c)
     -- info_mach_region_command (darwin-nat-info.c)
     -- parse_and_eval_address
     --- ada_unhandled_exception_name_addr (ada-lang.c)
     --- ada_unhandled_exception_name_addr_from_raise
     --- ada_exception_name_addr_1
     --- bsd_kvm_proc_cmd (bsd-kvm.c)
     --- bsd_kvm_pcb_cmd
     --- disassemble_command (cli-cmds.c)
     --- dump_memory_to_file (cli-dump.c)
     --- restore_command
     --- set_section_command
     --- set_task_exc_port_cmd (gnu-nat.c)
     --- set_thread_exc_port_cmd
     --- go32_pte_for_address (go32-nat.c)
     --- unwind_command (hppa-tdep.c)
     --- maintenance_translate_address (maint.c)
     --- mem_command (memattr.c)
     --- mi_cmd_disassemble (mi-cmd-disas.c)
     --- mi_cmd_data_write_register_values (mi-main.c)
     --- mi_cmd_data_read_memory
     --- mi_cmd_data_read_memory_bytes
     --- mi_cmd_data_write_memory
     --- mi_cmd_data_write_memory_bytes
     --- mi_cmd_trace_find
     --- sym_info (printcmd.c)
     --- add_symbol_file_command (symfile.c)
     --- add_symbol_file_from_memory_command (symfile-mem.c)
     --- trace_find_pc_command (tracepoint.c)
     --- trace_find_range_command
     --- trace_find_outside_command
     -- parse_and_eval_long
     --- breakpoint_1i (breakpoint.c)
     ---- breakpoints_info
     ---- watchpoints_info
     ---- maintenance_info_breakpoints
     ---- tracepoints_info
     --- do_set_command (cli-setshow.c)
     ---- execute_command (top.c)
     --- restore_command (cli-dump.c)
     --- go32_sldt (go32-nat.c)
     --- go32_sgdt
     --- go32_sidt
     --- go32_pde
     --- go32_pte
     --- continue_command (infcmd.c)
     --- step_1
     ---- step_command
     ---- next_command
     ---- stepi_command
     ---- nexti_command
     --- signal_command
     --- inferior_command (inferior.c)
     --- add_inferior_command
     --- clone_inferior_command
     --- signals_info (infrun.c)
     --- call_lseek (linux-fork.c)
     --- delete_checkpoint_command
     --- detach_checkpoint_command
     --- info_checkpoints_command
     --- restart_command
     --- maintenance_info_program_spaces_command (progspace.c)
     --- cmd_record_goto (record.c)
     ---- record_goto_bookmark
     ----- target_goto_bookmark (target.h)
     ------ goto_bookmark_command (reverse.c)
     --- backtrace_command_1 (stack.c)
     ---- backtrace_command
     ---- backtrace_full_command
     --- up_silently_base
     ---- up_silently_command
     ---- up_command
     --- down_silently_base
     ---- down_silently_command
     ---- down_command
     --- show_commands (top.c)
     --- trace_find_command (tracepoint.c)
     ---- trace_find_end_command
     ---- trace_find_start_command
     ---- tfind_1
     --- trace_find_tracepoint_command
     --- set_radix (valrpint.c)
     --- show_values (value.c)
     --- display_selectors (windows-nat.c)
     -- parse_and_eval
     --- print_ada_task_info (ada-tasks.c)
     ---- info_tasks_command
     --- info_task
     ---- info_tasks_command
     --- task_command_1
     ---- task_command
     --- ignore_command (breakpoint.c)
     --- dump_value_to_file (cli-dump.c)
     ---- dump_value_command
     ---- dump_srec_value
     ---- dump_ihex_value
     ---- dump_texhex_value
     ---- dump_binary_value
     ---- append_binary_value
     --- info_vtbl_command (cp-support.c)
     --- mi_cmd_trace_define_variable
     XXX gdbpy_parse_and_eval (python.c)
     --- parse_frame_specification_1 (stack.c)
     ---- parse_frame_specification
     ----- func_command
     ---- frame_info
     ---- select_frame_command
     ----- frame_command
     ------ return_command
     ----- return_command
     --- do_captured_thread_select (thread.c)
     ---- gdb_thread_select
     ----- thread_command
     --- quit_force (top.c)
     ---- quit_command (cli-cmds.c)
     ----- stdin_event_handler (event-top.c)
     ----- captured_command_loop (main.c)
     ---- handle_sigterm (event-top.c)
     ---- captured_main (main.c)
     ---- mi_cmd_gdb_exit (mi-main.c)
     ---- mi_execute_command (mi-main.c)
     ----- mi_execute_command_wrapper
     ------ mi_execute_command_input_handler
     XXXXXX mi_interpreter_exec
     --- trace_variable_command (tracepoint.c)
     --- display_tob (windows-tdep.c)
     -- parse_and_eval_type
     ---- safe_parse_type (gdbtypes.c)
     ----- check_stub_method
     -- mi_cmd_data_evaluate_expression (mi-main.c)
     -- print_object_command (objc-lang.c)
     -- print_command_1 (printcmd.c)
     --- print_command
     --- call_command
     -- output_command
     --- trace_dump_actions (tracepoint.c)
     -- set_command
     -- x_command
     -- display_command
     -- doo_one_display
     -- return_command (stack.c)
     -- whatis_exp (typeprint.c)
     -- maintenance_print_type
     -- init_if_undefined_command (value.c)
     - ada_read_renaming_var_value (ada-lang.c)
     - create_excep_cond_exprs (ada-lang.c)
     - agent_eval_command_one (ax-gdb.c)
     - maint_agent_print_command (ax-gdb.c)
     - set_breakpoint_condition (breakpoint.c)
     - update_watchpoint
     - parse_cmd_to_aexpr
       X Why cmd1 = cmdrest, parse_exp_1 (&cmd1), cmdrest = cmd1?
     - init_breakpoint_sal
     - find_condition_and_thread
     - watch_command_1
     - update_breakpoint_locations
     - parse_to_comma_and_eval (eval.c)
     - validate_actionline (tracepoint.c)
     -- check_tracepoint_command (breakpoint.c)
     -- trace_dump_command
    - varobj_create (varobj.c)

I have not attempted to constify the other parse_and_eval_* functions 
yet. I leave that for a follow-on patch.

That's all of them AFAICT. Anything further you want wrt to analyzing 
callers that I missed?

Keith


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-11 16:50       ` Keith Seitz
@ 2013-03-11 18:30         ` Tom Tromey
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2013-03-11 18:30 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Pedro Alves, gdb-patches@sourceware.org ml

Keith> Ok, that strdup, while not necessary, is not yet ready to be removed
Keith> until parse_and_eval is const, which I did not do. I thought that
Keith> might be a much more involved patch, and planned to do that
Keith> separately.

Ok, that's good enough for me.

Keith> That's all of them AFAICT. Anything further you want wrt to analyzing
Keith> callers that I missed?

Nope.

Tom


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-09  1:04           ` Keith Seitz
@ 2013-03-11 22:09             ` Keith Seitz
  2013-03-12 10:18               ` Pedro Alves
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2013-03-11 22:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches

[-- Attachment #1: Type: text/plain, Size: 2997 bytes --]

On 03/08/2013 05:04 PM, Keith Seitz wrote:
> On 03/08/2013 05:00 PM, Pedro Alves wrote:
>>
>> And confirmed this approach is valid.

Excellent. I've made your requested changes, and I've added Tom's 
requested python cleanup (it was much less entangled than I'd feared).

Did I miss anything? [It's possible -- looking at this stuff can get 
pretty monotonous and I'm prone to overlooking things that wouldn't 
normally escape my radar.]

Keith

2013-03-07  Keith Seitz  <keiths@redhat.com>

	* ada-lang.c (ada_read_renaming_var_value): Pass const
	pointer to expression string to parse_exp_1.
	(craete_excep_cond_exprs): Likewise.
	* ax-gdb.c (agent_eval_command_one): Likewise.
	(maint_agent_printf_command): Likewise.
	Constify much of the string handling/parsing.
	* breakpoint.c (set_breakpoint_condition): Pass const
	pointer to expression string to parse_exp_1.
	(update_watchpoint): Likewise.
	(parse_cmd_to_aexpr): Constify string handling.
	Pass const pointer to parse_exp_1.
	(init_breakpoint_sal): Pass const pointer to parse_exp_1.
	(find_condition_and_thread): Likewise.
	Make TOK const.
	(watch_command_1): Make ARG const.
	Constify string handling.
	Copy the expression string instead of changing the input
	string.
	(update_breakpoint_location): Pass const pointer to
	parse_exp_1.
	* eval.c (parse_and_eval_address): Make EXP const.
	(parse_to_comma_and_eval): Make EXPP const.
	(parse_and_eval): Make EXP const.
	* expression.h (parse_expression): Make argument const.
	(parse_exp_1): Make first argument const.
	* findcmd.c (parse_find_args): Treat ARGS as const.
	* linespec.c (parse_linespec): Pass const pointer to
	linespec_expression_to_pc.
	(linespec_expression_to_pc): Make EXP_PTR const.
	* parse.c (parse_exp_1): Make STRINGPTR const.
	Make a copy of the expression to pass to parse_exp_in_context until
	this whole interface can be constified.
	(parse_expression): Make STRING const.
	* printcmd.c (ui_printf): Treat ARG as const.
	Handle const strings.
	* tracepoint.c (validate_actionline): Pass const pointer to
	all calls to parse_exp_1.
	(encode_actions_1): Likewise.
	* value.h (parse_to_comma_and_eval): Make argument const.
	(parse_and_eval_address): Likewise.
	(parse_and_eval): Likewise.
	* varobj.c (varobj_create): Pass const pointer to parse_exp_1.
	(varobj_set_value): Likewise.
	* cli/cli-cmds.c (disassemble_command): Treat ARG as const and
	constify string handling.
	Pass const pointers to parse_and_eval_address and
	parse_to_comman_and_eval.
	* cli/cli-utils.c (skip_to_space): Rename to ...
	(skip_to_space_const): ... this. Handle const strings.
	* cli/cli-utils.h (skip_to_space): Turn into macro which invokes
	skip_to_space_const.
	(skip_to_space_const): Declare.
	* common/format.c (parse_format_string): Make ARG const.
	Handle const strings.
	* common/format.h (parse_format_string): Make ARG const.
	* gdbserver/ax.c (ax_printf): Make FORMAT const.
	* python/python.c (gdbpy_parse_and_eval): Do not make a copy
	of the expression string.

[-- Attachment #2: constify-parse_exp_1-2.patch --]
[-- Type: text/x-patch, Size: 28063 bytes --]

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.394
diff -u -p -r1.394 ada-lang.c
--- ada-lang.c	5 Mar 2013 21:15:34 -0000	1.394
+++ ada-lang.c	11 Mar 2013 21:57:22 -0000
@@ -4056,15 +4056,14 @@ static struct value *
 ada_read_renaming_var_value (struct symbol *renaming_sym,
 			     struct block *block)
 {
-  char *sym_name;
+  const char *sym_name;
   struct expression *expr;
   struct value *value;
   struct cleanup *old_chain = NULL;
 
-  sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
-  old_chain = make_cleanup (xfree, sym_name);
+  sym_name = SYMBOL_LINKAGE_NAME (renaming_sym);
   expr = parse_exp_1 (&sym_name, 0, block, 0);
-  make_cleanup (free_current_contents, &expr);
+  old_chain = make_cleanup (free_current_contents, &expr);
   value = evaluate_expression (expr);
 
   do_cleanups (old_chain);
@@ -11385,7 +11384,7 @@ create_excep_cond_exprs (struct ada_catc
       if (!bl->shlib_disabled)
 	{
 	  volatile struct gdb_exception e;
-	  char *s;
+	  const char *s;
 
 	  s = cond_string;
 	  TRY_CATCH (e, RETURN_MASK_ERROR)
Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.109
diff -u -p -r1.109 ax-gdb.c
--- ax-gdb.c	7 Mar 2013 00:48:25 -0000	1.109
+++ ax-gdb.c	11 Mar 2013 21:57:22 -0000
@@ -2608,6 +2608,7 @@ agent_eval_command_one (char *exp, int e
   struct cleanup *old_chain = 0;
   struct expression *expr;
   struct agent_expr *agent;
+  const char *arg;
 
   if (!eval)
     {
@@ -2616,14 +2617,15 @@ agent_eval_command_one (char *exp, int e
         exp = decode_agent_options (exp);
     }
 
-  if (!eval && strcmp (exp, "$_ret") == 0)
+  arg = exp;
+  if (!eval && strcmp (arg, "$_ret") == 0)
     {
       agent = gen_trace_for_return_address (pc, get_current_arch ());
       old_chain = make_cleanup_free_agent_expr (agent);
     }
   else
     {
-      expr = parse_exp_1 (&exp, pc, block_for_pc (pc), 0);
+      expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
       old_chain = make_cleanup (free_current_contents, &expr);
       if (eval)
 	agent = gen_eval_for_expr (pc, expr);
@@ -2716,8 +2718,8 @@ maint_agent_printf_command (char *exp, i
   struct expression *argvec[100];
   struct agent_expr *agent;
   struct frame_info *fi = get_current_frame ();	/* need current scope */
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
 
@@ -2733,7 +2735,7 @@ maint_agent_printf_command (char *exp, i
 
   cmdrest = exp;
 
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("Must start with a format string."));
@@ -2749,19 +2751,19 @@ maint_agent_printf_command (char *exp, i
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest != ',' && *cmdrest != 0)
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.744
diff -u -p -r1.744 breakpoint.c
--- breakpoint.c	7 Mar 2013 21:57:29 -0000	1.744
+++ breakpoint.c	11 Mar 2013 21:57:22 -0000
@@ -950,7 +950,7 @@ set_breakpoint_condition (struct breakpo
     }
   else
     {
-      char *arg = exp;
+      const char *arg = exp;
 
       /* I don't know if it matters whether this is the string the user
 	 typed in or the decompiled expression.  */
@@ -1759,7 +1759,7 @@ update_watchpoint (struct watchpoint *b,
 
   if (within_current_scope && reparse)
     {
-      char *s;
+      const char *s;
 
       if (b->exp)
 	{
@@ -2186,8 +2186,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   struct agent_expr *aexpr = NULL;
   struct cleanup *old_chain = NULL;
   volatile struct gdb_exception ex;
-  char *cmdrest;
-  char *format_start, *format_end;
+  const char *cmdrest;
+  const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
   struct gdbarch *gdbarch = get_current_arch ();
@@ -2199,7 +2199,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
 
   if (*cmdrest == ',')
     ++cmdrest;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (*cmdrest++ != '"')
     error (_("No format string following the location"));
@@ -2215,14 +2215,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   if (*cmdrest++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   if (!(*cmdrest == ',' || *cmdrest == '\0'))
     error (_("Invalid argument syntax"));
 
   if (*cmdrest == ',')
     cmdrest++;
-  cmdrest = skip_spaces (cmdrest);
+  cmdrest = skip_spaces_const (cmdrest);
 
   /* For each argument, make an expression.  */
 
@@ -2232,7 +2232,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
   nargs = 0;
   while (*cmdrest != '\0')
     {
-      char *cmd1;
+      const char *cmd1;
 
       cmd1 = cmdrest;
       expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
@@ -9103,7 +9103,8 @@ init_breakpoint_sal (struct breakpoint *
 
       if (b->cond_string)
 	{
-	  char *arg = b->cond_string;
+	  const char *arg = b->cond_string;
+
 	  loc->cond = parse_exp_1 (&arg, loc->address,
 				   block_for_pc (loc->address), 0);
 	  if (*arg)
@@ -9374,7 +9375,7 @@ invalid_thread_id_error (int id)
    If no thread is found, *THREAD is set to -1.  */
 
 static void
-find_condition_and_thread (char *tok, CORE_ADDR pc,
+find_condition_and_thread (const char *tok, CORE_ADDR pc,
 			   char **cond_string, int *thread, int *task,
 			   char **rest)
 {
@@ -9385,12 +9386,12 @@ find_condition_and_thread (char *tok, CO
 
   while (tok && *tok)
     {
-      char *end_tok;
+      const char *end_tok;
       int toklen;
-      char *cond_start = NULL;
-      char *cond_end = NULL;
+      const char *cond_start = NULL;
+      const char *cond_end = NULL;
 
-      tok = skip_spaces (tok);
+      tok = skip_spaces_const (tok);
 
       if ((*tok == '"' || *tok == ',') && rest)
 	{
@@ -9398,7 +9399,7 @@ find_condition_and_thread (char *tok, CO
 	  return;
 	}
 
-      end_tok = skip_to_space (tok);
+      end_tok = skip_to_space_const (tok);
 
       toklen = end_tok - tok;
 
@@ -9417,24 +9418,24 @@ find_condition_and_thread (char *tok, CO
 	  char *tmptok;
 
 	  tok = end_tok + 1;
-	  tmptok = tok;
-	  *thread = strtol (tok, &tok, 0);
+	  *thread = strtol (tok, &tmptok, 0);
 	  if (tok == tmptok)
 	    error (_("Junk after thread keyword."));
 	  if (!valid_thread_id (*thread))
 	    invalid_thread_id_error (*thread);
+	  tok = tmptok;
 	}
       else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
 	{
 	  char *tmptok;
 
 	  tok = end_tok + 1;
-	  tmptok = tok;
-	  *task = strtol (tok, &tok, 0);
+	  *task = strtol (tok, &tmptok, 0);
 	  if (tok == tmptok)
 	    error (_("Junk after task keyword."));
 	  if (!valid_task_id (*task))
 	    error (_("Unknown task %d."), *task);
+	  tok = tmptok;
 	}
       else if (rest)
 	{
@@ -10836,7 +10837,7 @@ is_masked_watchpoint (const struct break
                 hw_read:   watch read, 
 		hw_access: watch access (read or write) */
 static void
-watch_command_1 (char *arg, int accessflag, int from_tty,
+watch_command_1 (const char *arg, int accessflag, int from_tty,
 		 int just_location, int internal)
 {
   volatile struct gdb_exception e;
@@ -10845,12 +10846,12 @@ watch_command_1 (char *arg, int accessfl
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   struct frame_info *frame;
-  char *exp_start = NULL;
-  char *exp_end = NULL;
-  char *tok, *end_tok;
+  const char *exp_start = NULL;
+  const char *exp_end = NULL;
+  const char *tok, *end_tok;
   int toklen = -1;
-  char *cond_start = NULL;
-  char *cond_end = NULL;
+  const char *cond_start = NULL;
+  const char *cond_end = NULL;
   enum bptype bp_type;
   int thread = -1;
   int pc = 0;
@@ -10859,15 +10860,19 @@ watch_command_1 (char *arg, int accessfl
   int use_mask = 0;
   CORE_ADDR mask = 0;
   struct watchpoint *w;
+  char *expression;
+  struct cleanup *back_to;
 
   /* Make sure that we actually have parameters to parse.  */
   if (arg != NULL && arg[0] != '\0')
     {
-      char *value_start;
+      const char *value_start;
+
+      exp_end = arg + strlen (arg);
 
       /* Look for "parameter value" pairs at the end
 	 of the arguments string.  */
-      for (tok = arg + strlen (arg) - 1; tok > arg; tok--)
+      for (tok = exp_end - 1; tok > arg; tok--)
 	{
 	  /* Skip whitespace at the end of the argument list.  */
 	  while (tok > arg && (*tok == ' ' || *tok == '\t'))
@@ -10937,13 +10942,19 @@ watch_command_1 (char *arg, int accessfl
 
 	  /* Truncate the string and get rid of the "parameter value" pair before
 	     the arguments string is parsed by the parse_exp_1 function.  */
-	  *tok = '\0';
+	  exp_end = tok;
 	}
     }
+  else
+    exp_end = arg;
 
-  /* Parse the rest of the arguments.  */
+  /* Parse the rest of the arguments.  From here on out, everything
+     is in terms of a newly allocated string instead of the original
+     ARG.  */
   innermost_block = NULL;
-  exp_start = arg;
+  expression = savestring (arg, exp_end - arg);
+  back_to = make_cleanup (xfree, expression);
+  exp_start = arg = expression;
   exp = parse_exp_1 (&arg, 0, 0, 0);
   exp_end = arg;
   /* Remove trailing whitespace from the expression before saving it.
@@ -10989,8 +11000,8 @@ watch_command_1 (char *arg, int accessfl
   else if (val != NULL)
     release_value (val);
 
-  tok = skip_spaces (arg);
-  end_tok = skip_to_space (tok);
+  tok = skip_spaces_const (arg);
+  end_tok = skip_to_space_const (tok);
 
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
@@ -11142,6 +11153,7 @@ watch_command_1 (char *arg, int accessfl
     }
 
   install_breakpoint (internal, b, 1);
+  do_cleanups (back_to);
 }
 
 /* Return count of debug registers needed to watch the given expression.
@@ -14036,7 +14048,7 @@ update_breakpoint_locations (struct brea
 	 old symtab.  */
       if (b->cond_string != NULL)
 	{
-	  char *s;
+	  const char *s;
 	  volatile struct gdb_exception e;
 
 	  s = b->cond_string;
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.178
diff -u -p -r1.178 eval.c
--- eval.c	14 Feb 2013 12:43:45 -0000	1.178
+++ eval.c	11 Mar 2013 21:57:22 -0000
@@ -76,7 +76,7 @@ evaluate_subexp (struct type *expect_typ
    and return the result as a number.  */
 
 CORE_ADDR
-parse_and_eval_address (char *exp)
+parse_and_eval_address (const char *exp)
 {
   struct expression *expr = parse_expression (exp);
   CORE_ADDR addr;
@@ -104,7 +104,7 @@ parse_and_eval_long (char *exp)
 }
 
 struct value *
-parse_and_eval (char *exp)
+parse_and_eval (const char *exp)
 {
   struct expression *expr = parse_expression (exp);
   struct value *val;
@@ -121,7 +121,7 @@ parse_and_eval (char *exp)
    EXPP is advanced to point to the comma.  */
 
 struct value *
-parse_to_comma_and_eval (char **expp)
+parse_to_comma_and_eval (const char **expp)
 {
   struct expression *expr = parse_exp_1 (expp, 0, (struct block *) 0, 1);
   struct value *val;
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.47
diff -u -p -r1.47 expression.h
--- expression.h	1 Jan 2013 06:32:42 -0000	1.47
+++ expression.h	11 Mar 2013 21:57:22 -0000
@@ -95,12 +95,12 @@ struct expression
 
 /* From parse.c */
 
-extern struct expression *parse_expression (char *);
+extern struct expression *parse_expression (const char *);
 
 extern struct type *parse_expression_for_completion (char *, char **,
 						     enum type_code *);
 
-extern struct expression *parse_exp_1 (char **, CORE_ADDR pc,
+extern struct expression *parse_exp_1 (const char **, CORE_ADDR pc,
 				       const struct block *, int);
 
 /* For use by parsers; set if we want to parse an expression and
Index: findcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/findcmd.c,v
retrieving revision 1.23
diff -u -p -r1.23 findcmd.c
--- findcmd.c	8 Mar 2013 15:22:44 -0000	1.23
+++ findcmd.c	11 Mar 2013 21:57:22 -0000
@@ -69,7 +69,7 @@ parse_find_args (char *args, ULONGEST *m
   ULONGEST pattern_len;
   CORE_ADDR start_addr;
   ULONGEST search_space_len;
-  char *s = args;
+  const char *s = args;
   struct cleanup *old_cleanups;
   struct value *v;
 
@@ -110,7 +110,7 @@ parse_find_args (char *args, ULONGEST *m
 	    }
 	}
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   /* Get the search range.  */
@@ -120,7 +120,7 @@ parse_find_args (char *args, ULONGEST *m
 
   if (*s == ',')
     ++s;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s == '+')
     {
@@ -171,7 +171,7 @@ parse_find_args (char *args, ULONGEST *m
       struct type *t;
       ULONGEST pattern_buf_size_need;
 
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
 
       v = parse_to_comma_and_eval (&s);
       t = value_type (v);
@@ -219,7 +219,7 @@ parse_find_args (char *args, ULONGEST *m
 
       if (*s == ',')
 	++s;
-      s = skip_spaces (s);
+      s = skip_spaces_const (s);
     }
 
   if (pattern_buf_end == pattern_buf)
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.181
diff -u -p -r1.181 linespec.c
--- linespec.c	11 Mar 2013 18:24:59 -0000	1.181
+++ linespec.c	11 Mar 2013 21:57:22 -0000
@@ -326,7 +326,7 @@ static void iterate_over_file_blocks (st
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (char **exp_ptr);
+static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -2181,7 +2181,8 @@ parse_linespec (linespec_parser *parser,
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
   if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
     {
-      char *expr, *copy;
+      char *expr;
+      const char *copy;
 
       /* User specified an expression, *EXPR.  */
       copy = expr = copy_token_string (token);
@@ -2565,7 +2566,7 @@ initialize_defaults (struct symtab **def
    advancing EXP_PTR past any parsed text.  */
 
 static CORE_ADDR
-linespec_expression_to_pc (char **exp_ptr)
+linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
     /* The error message doesn't really matter, because this case
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.135
diff -u -p -r1.135 parse.c
--- parse.c	13 Jan 2013 18:57:00 -0000	1.135
+++ parse.c	11 Mar 2013 21:57:22 -0000
@@ -1125,10 +1125,18 @@ prefixify_subexp (struct expression *ine
    If COMMA is nonzero, stop if a comma is reached.  */
 
 struct expression *
-parse_exp_1 (char **stringptr, CORE_ADDR pc, const struct block *block,
+parse_exp_1 (const char **stringptr, CORE_ADDR pc, const struct block *block,
 	     int comma)
 {
-  return parse_exp_in_context (stringptr, pc, block, comma, 0, NULL);
+  struct expression *expr;
+  char *const_hack = *stringptr ? xstrdup (*stringptr) : NULL;
+  char *orig = const_hack;
+  struct cleanup *back_to = make_cleanup (xfree, const_hack);
+
+  expr = parse_exp_in_context (&const_hack, pc, block, comma, 0, NULL);
+  (*stringptr) += const_hack - orig;
+  do_cleanups (back_to);
+  return expr;
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
@@ -1264,7 +1272,7 @@ parse_exp_in_context (char **stringptr, 
    to use up all of the contents of STRING.  */
 
 struct expression *
-parse_expression (char *string)
+parse_expression (const char *string)
 {
   struct expression *exp;
 
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.223
diff -u -p -r1.223 printcmd.c
--- printcmd.c	11 Feb 2013 22:44:23 -0000	1.223
+++ printcmd.c	11 Mar 2013 21:57:22 -0000
@@ -2215,10 +2215,10 @@ printf_pointer (struct ui_file *stream, 
 /* printf "printf format string" ARG to STREAM.  */
 
 static void
-ui_printf (char *arg, struct ui_file *stream)
+ui_printf (const char *arg, struct ui_file *stream)
 {
   struct format_piece *fpieces;
-  char *s = arg;
+  const char *s = arg;
   struct value **val_args;
   int allocated_args = 20;
   struct cleanup *old_cleanups;
@@ -2229,7 +2229,7 @@ ui_printf (char *arg, struct ui_file *st
   if (s == 0)
     error_no_arg (_("format-control string and values to print"));
 
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   /* A format string should follow, enveloped in double quotes.  */
   if (*s++ != '"')
@@ -2242,14 +2242,14 @@ ui_printf (char *arg, struct ui_file *st
   if (*s++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
   
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   if (*s != ',' && *s != 0)
     error (_("Invalid argument syntax"));
 
   if (*s == ',')
     s++;
-  s = skip_spaces (s);
+  s = skip_spaces_const (s);
 
   {
     int nargs = 0;
@@ -2267,7 +2267,7 @@ ui_printf (char *arg, struct ui_file *st
 
     while (*s != '\0')
       {
-	char *s1;
+	const char *s1;
 
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.285
diff -u -p -r1.285 tracepoint.c
--- tracepoint.c	8 Mar 2013 15:35:06 -0000	1.285
+++ tracepoint.c	11 Mar 2013 21:57:22 -0000
@@ -754,9 +754,12 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
-	      exp = parse_exp_1 (&p, loc->address,
+	      const char *q;
+
+	      q = tmp_p;
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p = (char *) q;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      if (exp->elts[0].opcode == OP_VAR_VALUE)
@@ -806,10 +809,13 @@ validate_actionline (char **line, struct
 	  tmp_p = p;
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
-	      p = tmp_p;
+	      const char *q;
+
+	      q = tmp_p;
 	      /* Only expressions are allowed for this action.  */
-	      exp = parse_exp_1 (&p, loc->address,
+	      exp = parse_exp_1 (&q, loc->address,
 				 block_for_pc (loc->address), 1);
+	      p = (char *) q;
 	      old_chain = make_cleanup (free_current_contents, &exp);
 
 	      /* We have something to evaluate, make sure that the expr to
@@ -1470,9 +1476,12 @@ encode_actions_1 (struct command_line *a
 		  unsigned long addr;
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp = (char *) q;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  switch (exp->elts[0].opcode)
@@ -1561,9 +1570,12 @@ encode_actions_1 (struct command_line *a
 		{
 		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
+		  const char *q;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
+		  q = action_exp;
+		  exp = parse_exp_1 (&q, tloc->address,
 				     block_for_pc (tloc->address), 1);
+		  action_exp = (char *) q;
 		  old_chain = make_cleanup (free_current_contents, &exp);
 
 		  aexpr = gen_eval_for_expr (tloc->address, exp);
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.217
diff -u -p -r1.217 value.h
--- value.h	6 Feb 2013 19:40:04 -0000	1.217
+++ value.h	11 Mar 2013 21:57:22 -0000
@@ -724,13 +724,13 @@ extern char *extract_field_op (struct ex
 extern struct value *evaluate_subexp_with_coercion (struct expression *,
 						    int *, enum noside);
 
-extern struct value *parse_and_eval (char *exp);
+extern struct value *parse_and_eval (const char *exp);
 
-extern struct value *parse_to_comma_and_eval (char **expp);
+extern struct value *parse_to_comma_and_eval (const char **expp);
 
 extern struct type *parse_and_eval_type (char *p, int length);
 
-extern CORE_ADDR parse_and_eval_address (char *exp);
+extern CORE_ADDR parse_and_eval_address (const char *exp);
 
 extern LONGEST parse_and_eval_long (char *exp);
 
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.205
diff -u -p -r1.205 varobj.c
--- varobj.c	7 Mar 2013 19:24:32 -0000	1.205
+++ varobj.c	11 Mar 2013 21:57:22 -0000
@@ -620,7 +620,7 @@ varobj_create (char *objname,
       struct frame_info *fi;
       struct frame_id old_id = null_frame_id;
       struct block *block;
-      char *p;
+      const char *p;
       enum varobj_languages lang;
       struct value *value = NULL;
       volatile struct gdb_exception except;
@@ -1469,7 +1469,7 @@ varobj_set_value (struct varobj *var, ch
   struct expression *exp;
   struct value *value = NULL; /* Initialize to keep gcc happy.  */
   int saved_input_radix = input_radix;
-  char *s = expression;
+  const char *s = expression;
   volatile struct gdb_exception except;
 
   gdb_assert (varobj_editable_p (var));
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.150
diff -u -p -r1.150 cli-cmds.c
--- cli/cli-cmds.c	7 Mar 2013 21:57:30 -0000	1.150
+++ cli/cli-cmds.c	11 Mar 2013 21:57:24 -0000
@@ -1113,20 +1113,22 @@ disassemble_command (char *arg, int from
   const char *name;
   CORE_ADDR pc;
   int flags;
+  const char *p;
 
+  p = arg;
   name = NULL;
   flags = 0;
 
-  if (arg && *arg == '/')
+  if (p && *p == '/')
     {
-      ++arg;
+      ++p;
 
-      if (*arg == '\0')
+      if (*p == '\0')
 	error (_("Missing modifier."));
 
-      while (*arg && ! isspace (*arg))
+      while (*p && ! isspace (*p))
 	{
-	  switch (*arg++)
+	  switch (*p++)
 	    {
 	    case 'm':
 	      flags |= DISASSEMBLY_SOURCE;
@@ -1139,20 +1141,20 @@ disassemble_command (char *arg, int from
 	    }
 	}
 
-      arg = skip_spaces (arg);
+      p = skip_spaces_const (p);
     }
 
-  if (! arg || ! *arg)
+  if (! p || ! *p)
     {
       flags |= DISASSEMBLY_OMIT_FNAME;
       disassemble_current_function (flags);
       return;
     }
 
-  pc = value_as_address (parse_to_comma_and_eval (&arg));
-  if (arg[0] == ',')
-    ++arg;
-  if (arg[0] == '\0')
+  pc = value_as_address (parse_to_comma_and_eval (&p));
+  if (p[0] == ',')
+    ++p;
+  if (p[0] == '\0')
     {
       /* One argument.  */
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
@@ -1172,13 +1174,13 @@ disassemble_command (char *arg, int from
       /* Two arguments.  */
       int incl_flag = 0;
       low = pc;
-      arg = skip_spaces (arg);
-      if (arg[0] == '+')
+      p = skip_spaces_const (p);
+      if (p[0] == '+')
 	{
-	  ++arg;
+	  ++p;
 	  incl_flag = 1;
 	}
-      high = parse_and_eval_address (arg);
+      high = parse_and_eval_address (p);
       if (incl_flag)
 	high += low;
     }
Index: cli/cli-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.c,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.c
--- cli/cli-utils.c	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.c	11 Mar 2013 21:57:24 -0000
@@ -237,8 +237,8 @@ skip_spaces_const (const char *chp)
 
 /* See documentation in cli-utils.h.  */
 
-char *
-skip_to_space (char *chp)
+const char *
+skip_to_space_const (const char *chp)
 {
   if (chp == NULL)
     return NULL;
Index: cli/cli-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.h,v
retrieving revision 1.18
diff -u -p -r1.18 cli-utils.h
--- cli/cli-utils.h	12 Feb 2013 19:03:55 -0000	1.18
+++ cli/cli-utils.h	11 Mar 2013 21:57:24 -0000
@@ -101,7 +101,9 @@ extern const char *skip_spaces_const (co
 /* Skip leading non-whitespace characters in INP, returning an updated
    pointer.  If INP is NULL, return NULL.  */
 
-extern char *skip_to_space (char *inp);
+#define skip_to_space(INP) ((char *) skip_to_space_const ((INP)))
+
+extern const char *skip_to_space_const (const char *inp);
 
 /* Reverse S to the last non-whitespace character without skipping past
    START.  */
Index: common/format.c
===================================================================
RCS file: /cvs/src/src/gdb/common/format.c,v
retrieving revision 1.3
diff -u -p -r1.3 format.c
--- common/format.c	8 Feb 2013 22:52:20 -0000	1.3
+++ common/format.c	11 Mar 2013 21:57:24 -0000
@@ -28,11 +28,12 @@
 #include "format.h"
 
 struct format_piece *
-parse_format_string (char **arg)
+parse_format_string (const char **arg)
 {
-  char *s, *f, *string;
-  char *prev_start;
-  char *percent_loc;
+  const char *s;
+  char *f, *string;
+  const char *prev_start;
+  const char *percent_loc;
   char *sub_start, *current_substring;
   struct format_piece *pieces;
   int next_frag;
Index: common/format.h
===================================================================
RCS file: /cvs/src/src/gdb/common/format.h,v
retrieving revision 1.2
diff -u -p -r1.2 format.h
--- common/format.h	1 Jan 2013 06:32:54 -0000	1.2
+++ common/format.h	11 Mar 2013 21:57:24 -0000
@@ -51,7 +51,7 @@ struct format_piece
 /* Return an array of printf fragments found at the given string, and
    rewrite ARG with a pointer to the end of the format string.  */
 
-extern struct format_piece *parse_format_string (char **arg);
+extern struct format_piece *parse_format_string (const char **arg);
 
 /* Given a pointer to an array of format pieces, free any memory that
    would have been allocated by parse_format_string.  */
Index: gdbserver/ax.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/ax.c,v
retrieving revision 1.6
diff -u -p -r1.6 ax.c
--- gdbserver/ax.c	18 Jan 2013 06:40:57 -0000	1.6
+++ gdbserver/ax.c	11 Mar 2013 21:57:24 -0000
@@ -798,10 +798,10 @@ compile_bytecodes (struct agent_expr *ae
    in.  */
 
 static void
-ax_printf (CORE_ADDR fn, CORE_ADDR chan, char *format,
+ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
 	   int nargs, ULONGEST *args)
 {
-  char *f = format;
+  const char *f = format;
   struct format_piece *fpieces;
   int i, fp;
   char *current_substring;
Index: python/python.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python.c,v
retrieving revision 1.110
diff -u -p -r1.110 python.c
--- python/python.c	7 Mar 2013 21:57:30 -0000	1.110
+++ python/python.c	11 Mar 2013 21:57:25 -0000
@@ -720,11 +720,7 @@ gdbpy_parse_and_eval (PyObject *self, Py
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      char *copy = xstrdup (expr_str);
-      struct cleanup *cleanup = make_cleanup (xfree, copy);
-
-      result = parse_and_eval (copy);
-      do_cleanups (cleanup);
+      result = parse_and_eval (expr_str);
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 

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

* Re: [RFA] "constify" parse_exp_1
  2013-03-11 22:09             ` Keith Seitz
@ 2013-03-12 10:18               ` Pedro Alves
  2013-03-12 17:40                 ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2013-03-12 10:18 UTC (permalink / raw)
  To: Keith Seitz; +Cc: GDB Patches

On 03/11/2013 10:09 PM, Keith Seitz wrote:

> 2013-03-07  Keith Seitz  <keiths@redhat.com>
> 
>     * ada-lang.c (ada_read_renaming_var_value): Pass const
>     pointer to expression string to parse_exp_1.
>     (craete_excep_cond_exprs): Likewise.

s/craete/create

>     * ax-gdb.c (agent_eval_command_one): Likewise.
>     (maint_agent_printf_command): Likewise.
>     Constify much of the string handling/parsing.
>     * breakpoint.c (set_breakpoint_condition): Pass const
>     pointer to expression string to parse_exp_1.
>     (update_watchpoint): Likewise.
>     (parse_cmd_to_aexpr): Constify string handling.
>     Pass const pointer to parse_exp_1.
>     (init_breakpoint_sal): Pass const pointer to parse_exp_1.
>     (find_condition_and_thread): Likewise.
>     Make TOK const.
>     (watch_command_1): Make ARG const.

(Minor note: pedantically, uppercase is used when talking
about the value of the variable, rather than the variable
itself.)


> --- cli/cli-utils.h	12 Feb 2013 19:03:55 -0000	1.18
> +++ cli/cli-utils.h	11 Mar 2013 21:57:24 -0000
> @@ -101,7 +101,9 @@ extern const char *skip_spaces_const (co
>  /* Skip leading non-whitespace characters in INP, returning an updated
>     pointer.  If INP is NULL, return NULL.  */
>
> -extern char *skip_to_space (char *inp);
> +#define skip_to_space(INP) ((char *) skip_to_space_const ((INP)))

Double (())s not necessary:

  #define skip_to_space(INP) ((char *) skip_to_space_const (INP))

Can you please add:

  + /* A const-correct version of the above.  */

  extern const char *skip_to_space_const (const char *inp);

Just like skip_spaces_const has too.

Otherwise this all looks good to me.

-- 
Pedro Alves


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

* Re: [RFA] "constify" parse_exp_1
  2013-03-12 10:18               ` Pedro Alves
@ 2013-03-12 17:40                 ` Keith Seitz
  0 siblings, 0 replies; 13+ messages in thread
From: Keith Seitz @ 2013-03-12 17:40 UTC (permalink / raw)
  To: GDB Patches

On 03/12/2013 03:18 AM, Pedro Alves wrote:
> s/craete/create

Fixed.

> (Minor note: pedantically, uppercase is used when talking
> about the value of the variable, rather than the variable
> itself.)

Fixed/changed.

>> -extern char *skip_to_space (char *inp);
>> +#define skip_to_space(INP) ((char *) skip_to_space_const ((INP)))
>
> Double (())s not necessary:

Fixed.

>
>    #define skip_to_space(INP) ((char *) skip_to_space_const (INP))
>
> Can you please add:
>
>    + /* A const-correct version of the above.  */
>

Fixed.

> Otherwise this all looks good to me.
>

Committed. Thank you for taking a look at this.

Keith


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

end of thread, other threads:[~2013-03-12 17:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-08  0:12 [RFA] "constify" parse_exp_1 Keith Seitz
2013-03-08  0:22 ` Pedro Alves
2013-03-08  1:02   ` Keith Seitz
2013-03-08 12:27     ` Pedro Alves
2013-03-08 13:55       ` Pedro Alves
2013-03-09  1:00         ` Pedro Alves
2013-03-09  1:04           ` Keith Seitz
2013-03-11 22:09             ` Keith Seitz
2013-03-12 10:18               ` Pedro Alves
2013-03-12 17:40                 ` Keith Seitz
2013-03-08 14:54     ` Tom Tromey
2013-03-11 16:50       ` Keith Seitz
2013-03-11 18:30         ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox