From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22007 invoked by alias); 22 Aug 2012 23:30:40 -0000 Received: (qmail 21997 invoked by uid 22791); 22 Aug 2012 23:30:39 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 22 Aug 2012 23:30:23 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7MNUMcF022680 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 22 Aug 2012 19:30:22 -0400 Received: from psique (ovpn-113-26.phx2.redhat.com [10.3.113.26]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q7MNUJE8007543; Wed, 22 Aug 2012 19:30:20 -0400 From: Sergio Durigan Junior To: Aaron Gamble Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [patch] info threads sort by name and name regex matching References: <87pq6isqt9.fsf@fleche.redhat.com> X-URL: http://www.redhat.com Date: Wed, 22 Aug 2012 23:30:00 -0000 In-Reply-To: (Aaron Gamble's message of "Wed, 22 Aug 2012 15:36:25 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 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: 2012-08/txt/msg00642.txt.bz2 On Wednesday, August 22 2012, Aaron Gamble wrote: > On Wed, Aug 22, 2012 at 11:51 AM, Tom Tromey wrote: >> Aaron> With this patch sorting by name will happen always. (Perhaps a switch >> Aaron> to enable sorting would be better?) >> >> Yes, I think so. I think sorting by name makes sense but it isn't >> perhaps always what you want. Sorting by number has the nice feature >> that it is stable. >> >> Aaron> Regex matching is specified by doing 'info threads r'. This is >> Aaron> not ambiguous with previous behavior where parameters to info threads >> Aaron> were only numbers. (spaces between 'r' and are ignored) >> >> I'm curious why you chose this particular spelling. >> Other possible approaches would be a subcommand, or a flag like "-r". >> Either of these is perhaps more in keeping with gdb tradition. >> >> I'm interested in other opinions here too. >> >> Expect some bikeshedding on this point. > I picked a single letter for it to be quick and easy to type. I > suppose it's possible 'info threads' could be expanded to do other > sorts of matching where different flags would be useful. > > How about 'info threads [ -a ] [ id.. | -n ]' > > -a - Sort alphabetically by name > -n regex - Match thread names with regex > > Of course then we could do sorting based on the name of the function a > thread is in or other sorts of sorting. So other sorting flags would > need to be introduced. Why not `-r regex'? I think it would be more clear. Other commands (`sharedlibrary', `info variables', etc) take a regex as their first argument, without requiring a modifier like `-r'. If the "pipe" patch were already in, this alphabetical sorting would not be needed... Anyway, just thinking here. > *snip* >> Aaron> + threads = xmalloc (sizeof (*threads) * thread_list_size); >> Aaron> + make_cleanup (free, threads); >> >> I think making a VEC here would be better. >> Then you wouldn't need thread_list_size at all. > > Hmm, not sure if this would improve performance at all. A one time > allocation bounded by the number of threads vs VEC's implementation of > expanding arrays. I'll wait for others feedback. I guess Tom did not suggest this because of performance per se, but rather because if you have to make a list in GDB then it is already a convention to use VEC for these things. BTW, I noticed one minor nit in your patch: you do not obey the "TAB vs. space" rule. Basically, if you have 8 spaces, then you should convert it to a TAB. Emacs does this for free, but if you're using Vim you can ask me offlist and I can send you a little function that does that. Thanks, -- Sergio