This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA/Ada] Implement Ada tasking support
- From: Tom Tromey <tromey at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 23 Sep 2008 13:20:32 -0600
- Subject: Re: [RFA/Ada] Implement Ada tasking support
- References: <20080922233209.GE24389@adacore.com>
- Reply-to: tromey at redhat dot com
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> 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