From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29560 invoked by alias); 15 Nov 2008 21:30:55 -0000 Received: (qmail 29437 invoked by uid 22791); 15 Nov 2008 21:30:54 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 15 Nov 2008 21:30:18 +0000 Received: (qmail 28251 invoked from network); 15 Nov 2008 21:30:16 -0000 Received: from unknown (HELO localhost) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Nov 2008 21:30:16 -0000 From: Vladimir Prus To: Michael Snyder Subject: Re: [RFC] Implement -list-thread-groups. Date: Sun, 16 Nov 2008 08:22:00 -0000 User-Agent: KMail/1.9.10 Cc: "gdb-patches@sources.redhat.com" References: <200811122333.29218.vladimir@codesourcery.com> <200811151159.49049.vladimir@codesourcery.com> <491F3B35.1030303@vmware.com> In-Reply-To: <491F3B35.1030303@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200811160030.17037.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00400.txt.bz2 On Sunday 16 November 2008 00:12:21 Michael Snyder wrote: > Vladimir Prus wrote: > > On Friday 14 November 2008 22:41:58 Michael Snyder wrote: > >> 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. > > > > This is not the question of what *external* inputs, or user-defined > > inputs can be meaningful. It's the question of what the function > > promises. In my original patch, the function, in its comment, did not > > say anything about behaviour in the case where both thread and pid > > are not -1. Therefore, any caller of this function that can possible > > pass thread!=-1 and pid!=-1 gets undefined behaviour. There are 3 ways > > from here: > > > > 1. Document that thread!=-1 && pid!=-1 is invalid parameter set of this function. > > Add gdb_assert. > > > > 2. Document, exactly, the behaviour in thread!=-1 && pid !=-1 case. > > > > 3. Leave everything as is -- e.g. with undefined behaviour. > > > > (3) is not good, for obvious reasons. If you don't like (1), then can you specify > > what behaviour you want from this function in the thread!=-1 && pid !=-1 case, > > so that I can document and implement it? > > Sounds good, and well summarized. > > What about this for #2: > 1) Look up the thread based on TID as you already do. > 2) Compare the thread's PID with the supplied PID. > 3) If they match, print the result. If not, return error / not found. > > Sound reasonable? The function does not have a return value, so 'error' is the only way to do reporting. Is that what you suggest? - Volodya