This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Implement -list-thread-groups.


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]