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.


Vladimir Prus wrote:
On Friday 14 November 2008 21:54:46 Michael Snyder wrote:

I'm puzzled by this assert.
You don't think we'll ever want to specify both the pid and the thread?
I think that makes no sense. If a thread is specified, then there's no
possible use of 'pid'. Threads are globally numbered.
Even if it makes no sense in the sense that
it's not required, that doesn't necessarily make it
an error.  Suppose somebody specifies both the pid and
the thread?  What's the harm?  If they're inconsistent
(this pid does not contain this thread), THEN we'll
return an error.

I think it's better to make functions have as tight preconditions as possible. In this case, passing both thread and pid does not serve any possible purpose,
so it's likely that caller is doing this by mistake. It's best to assert immediately, rather than spending time and code space verifying if those
parameters are consistent.

I respect your opinion, but MI is not the only caller of this function.


> Checking if a thread belongs to a process is not
the part of this this function purpose.

It's input validation. What you're doing is also input validation, it's just imposing a more stringent requirement.

I feel that an assert is excessively stringent in this context.
An assert implies an internal gdb error.  These potentially
conflicting inputs could come about as a result of (foreseeable)
user input, rather than internal error.  Admittedly not any
user input that could be given now, but the CLI (or other
potential clients) could change.

I feel that if it's possible for these inputs to violate
the assert without actually reflecting an internally
inconsistant state, then the assert is too strong.


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