From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29441 invoked by alias); 9 Jun 2009 00:49:07 -0000 Received: (qmail 29431 invoked by uid 22791); 9 Jun 2009 00:49:06 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from qw-out-1920.google.com (HELO qw-out-1920.google.com) (74.125.92.148) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 09 Jun 2009 00:49:01 +0000 Received: by qw-out-1920.google.com with SMTP id 4so1812628qwk.24 for ; Mon, 08 Jun 2009 17:48:58 -0700 (PDT) Received: by 10.224.37.77 with SMTP id w13mr7480665qad.142.1244508538619; Mon, 08 Jun 2009 17:48:58 -0700 (PDT) Received: from hydrogen.gmail.com (207-172-203-39.c3-0.upd-ubr7.trpr-upd.pa.cable.rcn.com [207.172.203.39]) by mx.google.com with ESMTPS id 7sm270448qwb.46.2009.06.08.17.48.57 (version=TLSv1/SSLv3 cipher=RC4-MD5); Mon, 08 Jun 2009 17:48:58 -0700 (PDT) Date: Tue, 09 Jun 2009 00:49:00 -0000 Message-ID: <87zlcicjuv.wl%naesten@gmail.com> From: Samuel Bronson To: tromey@redhat.com Cc: Samuel Bronson , gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] Add "set/show debug unwinder" prefix commands. In-Reply-To: References: <1243638987-4533-1-git-send-email-naesten@gmail.com> <1243638987-4533-2-git-send-email-naesten@gmail.com> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.7 Emacs/22.3 (i486-pc-linux-gnu) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII 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: 2009-06/txt/msg00198.txt.bz2 At Thu, 04 Jun 2009 16:05:06 -0600, Tom Tromey wrote: > > >>>>> "Samuel" == Samuel Bronson writes: > > Samuel> Also add one subcommand for tracing the sniffing of stack > Samuel> frames by unwinders. > > This is also basically ok; just needs a ChangeLog and some formatting > fixlets, nothing serious. > > Samuel> +/* trace unwinders called */ > Samuel> +static int trace_unwinders; > > In the GNU style, comments should be sentences, so should start with a > capital letter and end with a period followed by two spaces. Yes, > we're that nit picky ;) Strange definition of sentence you have at GNU. > There are a few instances of this. > > Samuel> + if (trace_unwinders) { > Samuel> + fprintf_unfiltered (gdb_stdlog, "[ Searching for unwinder for frame ...\n"); > Samuel> + } Point, even the Linux style guide says you only need those if you want to match the other body in an if/else, or are in the midst of an if/else-if/.../else chain... > Samuel> for (entry = table->list; entry != NULL; entry = entry->next) > Samuel> { > Samuel> struct cleanup *old_cleanup; > Samuel> + const char *uname = entry->unwinder->unwinder_name; > Samuel> + > Samuel> + if (trace_unwinders) { > > The opening brace is misplaced here, in the GNU style it goes on the > next line. There are a couple instances of this. Grrr... why did RMS have to abandon the prophets Kernighan and Ritchie? > Samuel> + add_setshow_boolean_cmd ("trace-tried", class_maintenance, &trace_unwinders, > > I think this could just be "set debug unwinder", rather than > "set debug unwinder trace-tried". > > If you do plan to send another "debug unwinder" sub-command, maybe we > could discuss the naming now, before the patch. Actually, I ended up deciding the one I was thinking of wouldn't be that useful, especially when I figured out it was the kernel link process that was causing my actual problem, so I guess "set debug unwinder" would be fine.