This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: make execute_control_command conform to docs
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;
> }