From: Michael Snyder <msnyder@vmware.com>
To: Vladimir Prus <vladimir@codesourcery.com>
Cc: "gdb-patches@sources.redhat.com" <gdb-patches@sources.redhat.com>
Subject: Re: [RFC] Implement -list-thread-groups.
Date: Sat, 15 Nov 2008 16:10:00 -0000 [thread overview]
Message-ID: <491DD486.1070407@vmware.com> (raw)
In-Reply-To: <200811142218.17090.vladimir@codesourcery.com>
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.
next prev parent reply other threads:[~2008-11-14 19:43 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 [this message]
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
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=491DD486.1070407@vmware.com \
--to=msnyder@vmware.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