From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19522 invoked by alias); 20 Sep 2014 18:39:15 -0000 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 Received: (qmail 19504 invoked by uid 89); 20 Sep 2014 18:39:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f179.google.com Received: from mail-vc0-f179.google.com (HELO mail-vc0-f179.google.com) (209.85.220.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sat, 20 Sep 2014 18:39:12 +0000 Received: by mail-vc0-f179.google.com with SMTP id la4so2698868vcb.10 for ; Sat, 20 Sep 2014 11:39:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to :content-type; bh=XF6k5lTT3Gg9tQ+VbiQat8t0nH8tERjLIzOdzZWriy0=; b=QdhEC6xlBpyA1/YxSv2xRZ1Zf9Sto4Da/+wSJSfYogolsqKDqsNPdhz305+ChI+6t9 jv2LoApT/uBvjeB52MaA/Vpx9wbDUcgUHTkA3L89KH/oHmfgKHdd0hXKyX/F9FQIp87D gtULSZRNm8+XDiZaNfpsiP7048Y2EU/Xi9CBQyqYOjkDS1fXrL31OQoswVpfxT5AZ2a+ RkHIhIjU5GZI8wSQV+X4xpKMrefADMtsTBsXB3cloU01wE9xyl20KLhQuwTa2DRYFxXR nVOXlPe+U+IZZqVLxCoFhIX1IcZDRQs8mTglOoPQPJ96ORmCFG2DhiDb3bKfDmt5DK5M h4fQ== X-Gm-Message-State: ALoCoQmPMBPwqkh0g+SuZybLhKF5sbxqfDGKbTifoRYqRhxW4n06fZl9ILHDammRON5c3gmXCz6f MIME-Version: 1.0 X-Received: by 10.220.116.196 with SMTP id n4mr10738927vcq.6.1411238350447; Sat, 20 Sep 2014 11:39:10 -0700 (PDT) Received: by 10.52.181.65 with HTTP; Sat, 20 Sep 2014 11:39:10 -0700 (PDT) Date: Sat, 20 Sep 2014 18:39:00 -0000 Message-ID: Subject: Questions on commit 6c95b8df7fef5273da71c34775918c554aae0ea8 From: Doug Evans To: gdb-patches , Pedro Alves , Stan Shebs Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00646.txt.bz2 [Ugh, bad To address on first try.] Hi. While looking into removing lwp_list from gdb (akin to what I did for gdbserver) I found that several bits of target code are calling init_thread_list (grep init_thread_list *.c). Maybe there's the odd case where target code would need to do this, but normally when should target code *ever* do this? [ side notes: - having to deal with target code doing common operations (for lack of a better phrase) is something I have to fix in another context (prologue skipping), so it's now a bit of a pet peeve of mine - it's likely there are more such cases (e.g. init_wait_for_inferior), but for the nonce I'm picking init_thread_list as a basis for discussion - these issues are not specific to or introduced by the commit specified in the subject line, it's just a concrete base from which to phrase my questions ] To try to assist you with getting a handle on my confusion, consider remote.c:extended_remote_create_inferior_1 from gdb 6.8: /* Clean up from the last time we were running. */ init_thread_list (); init_wait_for_inferior (); Now look at what's there today: if (!have_inferiors ()) { /* Clean up from the last time we ran, before we mark the target running again. This will mark breakpoints uninserted, and get_offsets may insert breakpoints. */ init_thread_list (); init_wait_for_inferior (); } I think(!) there may be multiple ways to look at all of this as being wrong, so pick your favorite, but here's one way: What does it matter whether there are other inferiors as to whether remote.c:extended_remote_create_inferior has to "clean up from the last time we were running"? Obviously we can't call init_thread_list if there are other inferiors, but why are we calling init_thread_list here at all? Why isn't this state being managed by gdb itself (inf*.c or some such)? I can imagine one can look at this as just being still a work in progress on the way to proper multi-target support. It's stil a bit odd though to have taken this step this way, so I'm hoping for some clarification. Another related question I have is: Why does remote-sim.c:gdbsim_create_inferior call init_wait_for_inferior unconditionally whereas the above code conditions the call on !have_inferiors()? Maybe it's a simple oversight, but I think (emphasis on THINK, I could certainly be missing something) we need to take a step back and ask why this code is there at all. Putting this code in target routines gives us a lot of flexibility, but the cost is more mental load (for lack of a better phrase) and more touch points when trying to fix/improve core gdb, and I'm getting the impression that the pendulum has swung too far towards putting general housekeeping operations in target code.