Re: make execute_control_command conform to docs

[Dave Allan <da_gdb at egenera dot com>]

That patch looks good to me.  I was also able to apply it to the gdb
tree in 'crash' where I originally discovered the problem, and the
segfault there is gone.  

Thanks for all the help!

Dave


On Tue, 2004-02-24 at 18:12, Andrew Cagney wrote:
> > 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;
>  }