From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14089 invoked by alias); 31 Oct 2010 21:07:48 -0000 Received: (qmail 14079 invoked by uid 22791); 31 Oct 2010 21:07:47 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 31 Oct 2010 21:07:42 +0000 Received: (qmail 29729 invoked from network); 31 Oct 2010 21:07:41 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 31 Oct 2010 21:07:41 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, pmuldoon@redhat.com Subject: Re: [patch] Add visible flag to breakpoints. Date: Sun, 31 Oct 2010 21:07:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.33-29-realtime; KDE/4.4.2; x86_64; ; ) Cc: Tom Tromey , dan@codesourcery.com References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201010312107.38336.pedro@codesourcery.com> X-IsSubscribed: yes 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: 2010-10/txt/msg00407.txt.bz2 On Friday 22 October 2010 22:05:30, Phil Muldoon wrote: > -/* Set a breakpoint. This function is shared between CLI and MI > +/* Set a breakpoint. This function is shared between CLI and MI Spurious, and incorrect whitespace change. > 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. Returns true if any breakpoint > - was created; false otherwise. */ > - > + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the > + breakpoint number will be allocated from the internal breakpoint > + count. Returns true if any breakpoint was created; false > + otherwise. */ > int Two spaces after period, not three. Don't lose the empty line between comment and function. > create_breakpoint (struct gdbarch *gdbarch, > @@ -7750,7 +7785,8 @@ break_command_1 (char *arg, int flag, int from_tty) > pending_break_support, > NULL /* breakpoint_ops */, > from_tty, > - 1 /* enabled */); > + 1 /* enabled */, > + 0 /* internal */); > } > Inconsistent whitespace with the other cases. Please fix that in all the several places this appears. > int static_trace_marker_id_idx; > - }; > + > + /* With a Python scripting enabled GDB, store a reference to the > + Python object that has been associated with this breakpoint. > + This is always NULL for a GDB that is not script enabled. It > + can sometimes be NULL for enabled GDBs as not all breakpoint > + types are tracked by the Python scripting API. */ > + PyObject *py_bp_object; > +}; Is the indentation correct here? Looks two spaces too much to the right. > + if (internal) > + /* Do not mention breakpoints with a negative number, but do > + notify observers. */ > + observer_notify_breakpoint_created (b->number); > + else > + mention (b); Same here. Please double check your patch for formatting -- there may be other cases I missed. > extern int create_breakpoint (struct gdbarch *gdbarch, char *arg, > - char *cond_string, int thread, > - int parse_condition_and_thread, > - int tempflag, enum bptype wanted_type, > - int ignore_count, > - enum auto_boolean pending_break_support, > - struct breakpoint_ops *ops, > - int from_tty, > - int enabled); > + char *cond_string, int thread, > + int parse_condition_and_thread, > + int tempflag, > + enum bptype type_wanted, > + int ignore_count, > + enum auto_boolean pending_break_support, > + struct breakpoint_ops *ops, > + int from_tty, > + int enabled, > + int internal); Leftover reindent from "create_new_breakpoint"? Doesn't look necessary or correct? Please undo. > create_breakpoint (python_gdbarch, > - spec, NULL, -1, > - 0, > - 0, bp_breakpoint, > - 0, > - AUTO_BOOLEAN_TRUE, > - NULL, 0, 1); > + spec, NULL, -1, > + 0, > + 0, bp_breakpoint, > + 0, > + AUTO_BOOLEAN_TRUE, > + NULL, 0, 1, internal_bp); Same here? Otherwise looks fine to me. Thanks. -- Pedro Alves