From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4493 invoked by alias); 8 Nov 2007 10:26:17 -0000 Received: (qmail 4475 invoked by uid 22791); 8 Nov 2007 10:26:15 -0000 X-Spam-Check-By: sourceware.org Received: from zigzag.lvk.cs.msu.su (HELO zigzag.lvk.cs.msu.su) (158.250.17.23) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 08 Nov 2007 10:26:11 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1Iq4aH-0001sl-LV for gdb-patches@sources.redhat.com; Thu, 08 Nov 2007 13:26:08 +0300 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA:32) (Exim 4.50) id 1Iq4aH-0001sb-5f for gdb-patches@sources.redhat.com; Thu, 08 Nov 2007 13:26:01 +0300 From: Vladimir Prus To: gdb-patches@sources.redhat.com Subject: MI/CLI breakpoint handling code duplication Date: Thu, 08 Nov 2007 10:26:00 -0000 User-Agent: KMail/1.9.6 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_3QuMH6tjX/yy9Mh" Message-Id: <200711081325.59918.ghost@cs.msu.su> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-11/txt/msg00159.txt.bz2 --Boundary-00=_3QuMH6tjX/yy9Mh Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 814 The code paths handling break insertion in MI and CLI feature undue code duplication. This patch makes extract the body of break_command_1 into separate function, makes it slightly more flexibile, and as result, both MI and CLI code paths merely forward to the new function. No regressions on x86, OK? - Volodya * breakpoint.c (break_command_really): New, copied from break_command_1. New parameters COND_STRING, THREAD PARSE_CONDITITION_AND_THREAD and PENDING_BREAK_SUPPORT. The previous FLAG parameter split into TEMPFLAG and HARDWAREFLAG. When PARSE_CONDITION_AND_THREAD is not set, duplicate the passed condition string. (struct captured_breakpoint_args): Remove (do_captured_breakpoint): Remove. (break_command_1): Relay to break_command_really. (gdb_breakpoint): Relay to break_command_really. --Boundary-00=_3QuMH6tjX/yy9Mh Content-Type: text/x-diff; charset="utf-8"; name="pending_mi_2_code_duplication.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pending_mi_2_code_duplication.diff" Content-length: 8422 --- gdb/breakpoint.c (/patches/pending_mi_1_cleanup) (revision 43) +++ gdb/breakpoint.c (/patches/pending_mi_2_code_duplication) (revision 43) @@ -5435,18 +5435,26 @@ find_condition_and_thread (char *tok, CO } } -/* Set a breakpoint according to ARG (function, linenum or *address) - flag: first bit : 0 non-temporary, 1 temporary. - second bit : 0 normal breakpoint, 1 hardware breakpoint. */ +/* Set a breakpoint. This function is shared between + CLI and MI functions for setting a breakpoint. + This function has two major modes of operations, + selected by the PARSE_CONDITION_AND_THREAD parameter. + If non-zero, the function will parse arg, extracting + breakpoint location, address and thread. Otherwise, + ARG is just the location of breakpoint, with condition + and thread specified by the COND_STRING and THREAD + parameters. */ static int -break_command_1 (char *arg, int flag, int from_tty) +break_command_really (char *arg, char *cond_string, int thread, + int parse_condition_and_thread, + int tempflag, int hardwareflag, + enum auto_boolean pending_break_support, + int from_tty) { struct gdb_exception e; - int tempflag, hardwareflag; struct symtabs_and_lines sals; struct symtab_and_line pending_sal; - char *cond_string = NULL; char *copy_arg; char *err_msg; char *addr_start = arg; @@ -5456,13 +5464,9 @@ break_command_1 (char *arg, int flag, in struct captured_parse_breakpoint_args parse_args; int i; int pending = 0; - int thread = -1; int ignore_count = 0; int not_found = 0; - hardwareflag = flag & BP_HARDWAREFLAG; - tempflag = flag & BP_TEMPFLAG; - sals.sals = NULL; sals.nelts = 0; addr_string = NULL; @@ -5557,13 +5561,27 @@ break_command_1 (char *arg, int flag, in breakpoint. */ if (!pending) { - /* Here we only parse 'arg' to separate condition - from thread number, so parsing in context of first - sal is OK. When setting the breakpoint we'll - re-parse it in context of each sal. */ - find_condition_and_thread (arg, sals.sals[0].pc, &cond_string, &thread); - if (cond_string) - make_cleanup (xfree, cond_string); + if (parse_condition_and_thread) + { + /* Here we only parse 'arg' to separate condition + from thread number, so parsing in context of first + sal is OK. When setting the breakpoint we'll + re-parse it in context of each sal. */ + cond_string = NULL; + thread = -1; + find_condition_and_thread (arg, sals.sals[0].pc, &cond_string, &thread); + if (cond_string) + make_cleanup (xfree, cond_string); + } + else + { + /* Create a private copy of condition string. */ + if (cond_string) + { + cond_string = xstrdup (cond_string); + make_cleanup (xfree, cond_string); + } + } create_breakpoints (sals, addr_string, cond_string, hardwareflag ? bp_hardware_breakpoint : bp_breakpoint, @@ -5582,13 +5600,14 @@ break_command_1 (char *arg, int flag, in : bp_breakpoint); set_breakpoint_count (breakpoint_count + 1); b->number = breakpoint_count; - b->thread = thread; + b->thread = -1; b->addr_string = addr_string[0]; - b->cond_string = cond_string; + b->cond_string = NULL; b->ignore_count = ignore_count; b->disposition = tempflag ? disp_del : disp_donttouch; b->from_tty = from_tty; - b->flag = flag; + b->flag = (hardwareflag ? BP_HARDWAREFLAG : 0) + | (tempflag ? BP_TEMPFLAG : 0); b->condition_not_parsed = 1; mention (b); } @@ -5605,119 +5624,37 @@ break_command_1 (char *arg, int flag, in return GDB_RC_OK; } -/* Set a breakpoint of TYPE/DISPOSITION according to ARG (function, - linenum or *address) with COND and IGNORE_COUNT. */ - -struct captured_breakpoint_args - { - char *address; - char *condition; - int hardwareflag; - int tempflag; - int thread; - int ignore_count; - }; - +/* Set a breakpoint. + ARG is a string describing breakpoint address, + condition, and thread. + FLAG specifies if a breakpoint is hardware on, + and if breakpoint is temporary, using BP_HARDWARE_FLAG + and BP_TEMPFLAG. */ + static int -do_captured_breakpoint (struct ui_out *uiout, void *data) +break_command_1 (char *arg, int flag, int from_tty) { - struct captured_breakpoint_args *args = data; - struct symtabs_and_lines sals; - struct expression **cond; - struct cleanup *old_chain; - struct cleanup *breakpoint_chain = NULL; - int i; - char **addr_string; - char *cond_string = 0; - - char *address_end; - - /* Parse the source and lines spec. Delay check that the expression - didn't contain trailing garbage until after cleanups are in - place. */ - sals.sals = NULL; - sals.nelts = 0; - address_end = args->address; - addr_string = NULL; - parse_breakpoint_sals (&address_end, &sals, &addr_string, 0); - - if (!sals.nelts) - return GDB_RC_NONE; - - /* Create a chain of things at always need to be cleaned up. */ - old_chain = make_cleanup (null_cleanup, 0); - - /* Always have a addr_string array, even if it is empty. */ - make_cleanup (xfree, addr_string); - - /* Make sure that all storage allocated to SALS gets freed. */ - make_cleanup (xfree, sals.sals); - - /* Allocate space for all the cond expressions. */ - cond = xcalloc (sals.nelts, sizeof (struct expression *)); - make_cleanup (xfree, cond); - - /* ----------------------------- SNIP ----------------------------- - Anything added to the cleanup chain beyond this point is assumed - to be part of a breakpoint. If the breakpoint create goes - through then that memory is not cleaned up. */ - breakpoint_chain = make_cleanup (null_cleanup, 0); - - /* Mark the contents of the addr_string for cleanup. These go on - the breakpoint_chain and only occure if the breakpoint create - fails. */ - for (i = 0; i < sals.nelts; i++) - { - if (addr_string[i] != NULL) - make_cleanup (xfree, addr_string[i]); - } - - /* Wait until now before checking for garbage at the end of the - address. That way cleanups can take care of freeing any - memory. */ - if (*address_end != '\0') - error (_("Garbage %s following breakpoint address"), address_end); - - /* Resolve all line numbers to PC's. */ - breakpoint_sals_to_pc (&sals, args->address); + int hardwareflag = flag & BP_HARDWAREFLAG; + int tempflag = flag & BP_TEMPFLAG; - if (args->condition != NULL) - { - cond_string = xstrdup (args->condition); - make_cleanup (xfree, cond_string); - } - - create_breakpoints (sals, addr_string, args->condition, - args->hardwareflag ? bp_hardware_breakpoint : bp_breakpoint, - args->tempflag ? disp_del : disp_donttouch, - args->thread, args->ignore_count, 0/*from-tty*/); - - /* That's it. Discard the cleanups for data inserted into the - breakpoint. */ - discard_cleanups (breakpoint_chain); - /* But cleanup everything else. */ - do_cleanups (old_chain); - return GDB_RC_OK; + return break_command_really (arg, + NULL, 0, 1 /* parse arg */, + tempflag, hardwareflag, + pending_break_support, from_tty); } + enum gdb_rc gdb_breakpoint (char *address, char *condition, int hardwareflag, int tempflag, int thread, int ignore_count, char **error_message) { - struct captured_breakpoint_args args; - args.address = address; - args.condition = condition; - args.hardwareflag = hardwareflag; - args.tempflag = tempflag; - args.thread = thread; - args.ignore_count = ignore_count; - if (catch_exceptions_with_msg (uiout, do_captured_breakpoint, &args, - error_message, RETURN_MASK_ALL) < 0) - return GDB_RC_FAIL; - else - return GDB_RC_OK; + return break_command_really (address, condition, thread, + 0 /* condition and thread are valid. */, + tempflag, hardwareflag, + AUTO_BOOLEAN_FALSE /* no pending. */, + 0); } --Boundary-00=_3QuMH6tjX/yy9Mh--