From: Joel Brobecker <brobecker@adacore.com>
To: Vladimir Prus <vladimir@codesourcery.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFC] Implement -list-thread-groups.
Date: Sun, 16 Nov 2008 01:14:00 -0000 [thread overview]
Message-ID: <20081115173425.GI12802@adacore.com> (raw)
In-Reply-To: <200811142028.43561.vladimir@codesourcery.com>
> Sorry if I'm trying to circumvent the rules. On the other hand, I
> believe I was told that MI boundaries are logical, not physical. In
> that sense, a change that clearly matters only for MI, and is actually
> posted for comments, is kind of a boundary case.
>
> Another example are observes. Although generic in nature, they are quickly
> becoming de-facto interface of GDB core with MI.
I agree there is no absolute rules. But I have to disagree with the
observers. Maybe many of them are being used by MI, but several others
are not. At AdaCore, we have some local VxWorks ports that we developped
which rely on oberver notifications as well. There are still a couple
of areas that need polishing, otherwise we'd have submitted this port
for inclusion as well.
> Done. Indicentally, I know how to configure Emacs to remove trailing
> whitespace on save. But pretty much every source file in gdb already
> has such lines. If somebody tell me how to make Emacs not add lines
> without trailing whitespace, while *not* changing existing lines, it
> would be gtreat.
Yeah - I try to be careful about not making it worse, but until we
have automated style checks at checkin time, I think it's a lost
battle.
> + If PID is not -1, only prints threads from the process PID. */
^^^^^^ print
Since Michael commented on the following assertion:
> + gdb_assert (requested_thead == -1 || pid == -1);
^^^^^ Ooops, typo???
I think it's important to explain it in the function description.
Can you add a line or two explaining at the end of the function
description explaining that it doesn't make sense to provide both
the pid and the thread at the same time, and thus at least one
of the two must be set to 1.
The rest looks good to me. I don't remember if I pre-approved the patch
the previous time I reviewed your patch, so I'll just do it again now.
--
Joel
next prev parent reply other threads:[~2008-11-15 17:35 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-12 21:01 Vladimir Prus
2008-11-14 11:46 ` Joel Brobecker
2008-11-14 11:58 ` Michael Snyder
2008-11-14 19:43 ` Vladimir Prus
2008-11-14 19:44 ` Michael Snyder
2008-11-14 21:45 ` Vladimir Prus
2008-11-15 4:58 ` Michael Snyder
2008-11-15 9:00 ` Vladimir Prus
2008-11-15 16:10 ` Michael Snyder
2008-11-15 19:06 ` Vladimir Prus
2008-11-16 8:22 ` Michael Snyder
2008-11-16 8:22 ` Vladimir Prus
[not found] ` <29E9E827072C404C88A05DDC42B45997199E0503FF@PA-EXMBX14.vmware.com>
2008-11-17 9:42 ` Vladimir Prus
2008-11-17 19:48 ` Michael Snyder
2008-11-17 22:02 ` Vladimir Prus
2008-11-14 20:46 ` Pedro Alves
2008-11-16 1:14 ` Joel Brobecker [this message]
2008-11-16 8:20 ` Joel Brobecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081115173425.GI12802@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sources.redhat.com \
--cc=vladimir@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox