From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32072 invoked by alias); 23 Sep 2008 19:22:15 -0000 Received: (qmail 32063 invoked by uid 22791); 23 Sep 2008 19:22:14 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 23 Sep 2008 19:21:40 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id m8NJLHsd017782; Tue, 23 Sep 2008 15:21:17 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m8NJLGSf030718; Tue, 23 Sep 2008 15:21:16 -0400 Received: from opsy.redhat.com (vpn-10-159.bos.redhat.com [10.16.10.159]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id m8NJLFss009898; Tue, 23 Sep 2008 15:21:15 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id D6BAD508592; Tue, 23 Sep 2008 13:20:32 -0600 (MDT) To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [RFA/Ada] Implement Ada tasking support References: <20080922233209.GE24389@adacore.com> From: Tom Tromey Reply-To: tromey@redhat.com X-Attribution: Tom Date: Tue, 23 Sep 2008 19:22:00 -0000 In-Reply-To: <20080922233209.GE24389@adacore.com> (Joel Brobecker's message of "Mon\, 22 Sep 2008 16\:32\:09 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2008-09/txt/msg00484.txt.bz2 >>>>> "Joel" == Joel Brobecker writes: Hi Joel. I have a couple "big" things to comment on about this patch, plus a couple nit-picky things. Joel> * info tasks: Joel> * info task TASKNO: Joel> * task [TASKNO]: These names are all generic, but the feature is specific to Ada. How about "info ada task" and "ada task" as the commands instead? I am curious to know how this will interact with the "set" stuff coming with Stan's multi-exec work. Joel> I have omitted the part that allows you to restrict a breakpoint to Joel> a given task ("break LINESPEC task TASKNO") for now, but will soon Joel> start discussing this if this goes in. How about using convenience functions instead of new syntax? break LINESPEC if $current_ada_task() == 5 ... unless, I suppose, there's some strong reason to prefer having this information in the gdb internals. Now for the nit picks. I only looked at this superficially, I'm afraid. Joel> +#define target_get_ada_task_ptid(lwp, tid) \ Joel> + (*current_target.to_get_ada_task_ptid) (lwp,tid) Not sure if this kind of macro needs a comment? Joel> +ptid_t Joel> +default_get_ada_task_ptid (long lwp, long tid) Comment needed. Joel> +/* A short description corresponding to each possible task state. */ Joel> +static char *task_states[] = { Joel> + "Unactivated", I think these should be marked N_() and then run through gettext when printing. Likewise for long_task_states. Joel> +/* Set to 1 when the cached address of System.Tasking.Debug.Known_Tasks Joel> + might be stale and so needs to be recomputed. */ Joel> +static int ada_tasks_check_symbol_table = 1; For my own clarification -- is a blank line needed between the comment and the definition here? Or is that only for functions? (If needed, there are a few cases of this.) Joel> +void Joel> +iterate_over_live_ada_tasks (ada_task_list_iterator_ftype *iterator) Comment needed. Joel> +static ptid_t Joel> +thread_db_get_ada_task_ptid (long lwp, long thread) Likewise. Tom