From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15180 invoked by alias); 25 Feb 2004 14:02:43 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15126 invoked from network); 25 Feb 2004 14:02:41 -0000 Received: from unknown (HELO coyote.egenera.com) (63.160.166.46) by sources.redhat.com with SMTP; 25 Feb 2004 14:02:41 -0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by coyote.egenera.com (Postfix) with ESMTP id 1B178A0520; Wed, 25 Feb 2004 08:48:45 -0500 (EST) Received: from coyote.egenera.com ([127.0.0.1]) by localhost (coyote.egenera.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 00733-01; Wed, 25 Feb 2004 08:48:41 -0500 (EST) Received: from hasufel.egenera.com (southeast.egenera.com [63.160.166.4]) by coyote.egenera.com (Postfix) with ESMTP id 839CEA051A; Wed, 25 Feb 2004 08:48:41 -0500 (EST) Received: from localhost (localhost.localdomain [127.0.0.1]) by hasufel.egenera.com (8.11.6/8.11.6) with ESMTP id i1PDvok01433; Wed, 25 Feb 2004 08:57:50 -0500 Subject: Re: make execute_control_command conform to docs From: Dave Allan Reply-To: da_gdb@egenera.com To: Andrew Cagney Cc: GDB patches In-Reply-To: <403BDA4F.9000302@gnu.org> References: <1077640948.1311.61.camel@hasufel.egenera.com> <403BB5AD.9040502@gnu.org> <1077659195.1310.192.camel@hasufel.egenera.com> <403BDA4F.9000302@gnu.org> Content-Type: text/plain Organization: Egenera, Inc. Message-Id: <1077717470.1270.40.camel@hasufel.egenera.com> Mime-Version: 1.0 Date: Wed, 25 Feb 2004 14:02:00 -0000 Content-Transfer-Encoding: 7bit X-Virus-Scanned: by amavisd-new at egenera.com X-SW-Source: 2004-02/txt/msg00726.txt.bz2 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 > > 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; > }