From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18231 invoked by alias); 24 Mar 2008 07:18:22 -0000 Received: (qmail 18219 invoked by uid 22791); 24 Mar 2008 07:18:20 -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; Mon, 24 Mar 2008 07:18:01 +0000 Received: (qmail 30019 invoked from network); 24 Mar 2008 07:17:59 -0000 Received: from unknown (HELO localhost) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Mar 2008 07:17:59 -0000 From: Vladimir Prus To: Nick Roberts Subject: Re: Thread bound variable objects [was: Re: MI non-stop mode spec] Date: Mon, 24 Mar 2008 14:38:00 -0000 User-Agent: KMail/1.9.6 (enterprise 0.20070907.709405) Cc: gdb@sources.redhat.com References: <200803190016.02072.vladimir@codesourcery.com> <200803240843.54604.vladimir@codesourcery.com> <18407.20692.28713.106315@kahikatea.snap.net.nz> In-Reply-To: <18407.20692.28713.106315@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200803241017.57891.vladimir@codesourcery.com> Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2008-03/txt/msg00201.txt.bz2 On Monday 24 March 2008 09:57:24 Nick Roberts wrote: > > > OK, it should be: > > > > > > + if (old_cleanups != NULL) > > > + do_cleanups (old_cleanups); > > > > I think that's also wrong. In the event that no cleanups were installed > > before calling this function, this code will fail to run the > > cleanups installed by this function. In the event that a cleanup > > should be really installed conditionally, the right code is: > > > > struct cleanups *back_to = make_cleanup (null_cleanup, NULL); > > > > if (...) > > make_cleanup (); > > > > do_cleanups (back_to); > > I don't really see that: if nothing is added to cleanup_chain then no cleanups > are done. Suppose that no cleanups were installed before this function is called. Then, the first call to make_cleanup will return NULL, which will be stored in 'old_cleanups'. Then, at the end of the function you will execute: if (old_cleanups != NULL) do_cleanups (old_cleanups); Here, old_cleanups is NULL, so cleanups added in the function are not run. > > > I see now from the ChangeLog that you've committed your own change without > > > posting to the list first or explaining what it does. > > > > Sorry, no: > > 1. I've checked in two changes, not one. > > 2. Both are posted to gdb-patches. > > OK, my mistake. I saw the first one. > > > 3. Both are general cleanups, and don't implement anything that your patch > > tries to implement. > > It's just that the second one includes changes in and overlaps with my patch so > I thought it was part of that thread. I'm not sure where that leaves things > now. I've started looking at your patch, and saw various cleanups that can be done first, which I've done. I'll now get back to your patch proper, and hopefully will get it in today. > > > My patch does two things: > > > > > > 1) It stops a variable object from being considered automatically out of > > > scope when the selected thread changes. > > > 2) It associates a thread-id field with the variable object so that the > > > front end can organise the display of watch expressions accordingly. > > > > > > AFAICS your patch does neither of these. Could you please say what it > > > does do? > > > > Please see my gdb-patches posts. > > With regard to the second patch, why you are using > make_cleanup_restore_current_thread when you aren't switching the thread in the > first place? Because make_cleanup_restore_current_thread is handy pre-existing solution that will restore both thread and frame, and won't cause any harm if the thread is not actually changed. I probably could have introduced another cleanup to restore just frame, but that will be less safe, and only insignificantly faster. - Volodya