This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: make execute_control_command conform to docs


I was trying to make the patch as small as possible, but I ought to have
done it as you say. Letting everything flow through to the end so that
there is only one return point is definitely the cleanest way to do it.

Here, yes, it appears to be the case. Any way, does the attached appear to work?


Andrew

2004-02-24  Andrew Cagney  <cagney@redhat.com>

	PR cli/1566.  Problem found, and fix suggested by David Allan. 
	* cli/cli-script.c (execute_control_command): Unconditionally
	install a cleanup.  Default "ret" to "invalid_control".  Use
	"break" instead of "return" to escape from the switch.

Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.21
diff -u -r1.21 cli-script.c
--- cli/cli-script.c	22 Dec 2003 03:43:19 -0000	1.21
+++ cli/cli-script.c	24 Feb 2004 23:10:13 -0000
@@ -294,21 +294,25 @@
 {
   struct expression *expr;
   struct command_line *current;
-  struct cleanup *old_chain = 0;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, 0);
   struct value *val;
   struct value *val_mark;
   int loop;
   enum command_control_type ret;
   char *new_line;
 
+  /* Start by assuming failure, if a problem is detected, the code
+     below will simply "break" out of the switch.  */
+  ret = invalid_control;
+
   switch (cmd->control_type)
     {
     case simple_control:
       /* A simple command, execute it and return.  */
       new_line = insert_args (cmd->line);
       if (!new_line)
-	return invalid_control;
-      old_chain = make_cleanup (free_current_contents, &new_line);
+	break;
+      make_cleanup (free_current_contents, &new_line);
       execute_command (new_line, 0);
       ret = cmd->control_type;
       break;
@@ -325,8 +329,8 @@
 	/* Parse the loop control expression for the while statement.  */
 	new_line = insert_args (cmd->line);
 	if (!new_line)
-	  return invalid_control;
-	old_chain = make_cleanup (free_current_contents, &new_line);
+	  break;
+	make_cleanup (free_current_contents, &new_line);
 	expr = parse_expression (new_line);
 	make_cleanup (free_current_contents, &expr);
 
@@ -385,8 +389,8 @@
       {
 	new_line = insert_args (cmd->line);
 	if (!new_line)
-	  return invalid_control;
-	old_chain = make_cleanup (free_current_contents, &new_line);
+	  break;
+	make_cleanup (free_current_contents, &new_line);
 	/* Parse the conditional for the if statement.  */
 	expr = parse_expression (new_line);
 	make_cleanup (free_current_contents, &expr);
@@ -424,11 +428,10 @@
 
     default:
       warning ("Invalid control type in command structure.");
-      return invalid_control;
+      break;
     }
 
-  if (old_chain)
-    do_cleanups (old_chain);
+  do_cleanups (old_chain);
 
   return ret;
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]