From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11606 invoked by alias); 15 Sep 2014 16:00:45 -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 11558 invoked by uid 89); 15 Sep 2014 16:00:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 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-f175.google.com Received: from mail-vc0-f175.google.com (HELO mail-vc0-f175.google.com) (209.85.220.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 15 Sep 2014 16:00:42 +0000 Received: by mail-vc0-f175.google.com with SMTP id hq11so3614203vcb.34 for ; Mon, 15 Sep 2014 09:00:40 -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:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=KGqs6UlYPwlUPmU8KTUFAp0VN1kB/vuZxWWWIbWj4/c=; b=eA/4C2L4bx/+ttxoBhsVfg0bseAxjHeOK367SsN17Ayqc6iZs2sHK1917gfeG0z4vL 1TSWgQi0ICVEqVsDXtAgxbYC7zsoSr/mQ3MjdlA8nJGGALUDbcqRVfli83AQ2K4AERt1 ptNc3eSgCsvOEoVobqtQ4uXmNNnRitbLNMopZ3X2UeeX2Tc8+vHpSXfBDbe+3ZNpZz8D PkMLPkTAEoFpobNoCWAu7Jhet/dQuL3zOJ7CBJMnmlVmHWD6If/VzUcI56A6CrkzwxdS k3YSbh1TSEQ5D22cGbPULBuNeGgOiIkMpNpXAZcMiDJI3ey+LfhglMxUgx3RebwbUshD q1+w== X-Gm-Message-State: ALoCoQlNOAP9x69Gi3Znoc+B8tAOQJOaHCpS0VC9ylBWGLGzLdEDnQZcoKvsRD0/XLD5V9IySS63 MIME-Version: 1.0 X-Received: by 10.52.139.106 with SMTP id qx10mr20237773vdb.1.1410796840187; Mon, 15 Sep 2014 09:00:40 -0700 (PDT) Received: by 10.52.181.65 with HTTP; Mon, 15 Sep 2014 09:00:40 -0700 (PDT) In-Reply-To: <20140915100736.GA13503@blade.nx> References: <21520.36381.756875.963606@ruffy2.mtv.corp.google.com> <20140911102659.GA17472@blade.nx> <5412DEB5.6020706@redhat.com> <21523.9502.168492.803068@ruffy2.mtv.corp.google.com> <54132B55.9000108@redhat.com> <21523.12189.134570.770432@ruffy2.mtv.corp.google.com> <5413305B.6020402@redhat.com> <21523.13993.986533.615240@ruffy2.mtv.corp.google.com> <54133939.70801@redhat.com> <20140915100736.GA13503@blade.nx> Date: Mon, 15 Sep 2014 16:00:00 -0000 Message-ID: Subject: Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid From: Doug Evans To: Gary Benson Cc: Pedro Alves , gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00504.txt.bz2 On Mon, Sep 15, 2014 at 3:07 AM, Gary Benson wrote: > Doug Evans wrote: >> On Fri, Sep 12, 2014 at 11:19 AM, Pedro Alves wrote: >> > On 09/12/2014 07:08 PM, Doug Evans wrote: >> > > Pedro Alves wrote: >> > > > I just now noticed the elephant in the room -- target_stop >> > > > is asynchronous, doesn't wait for a stop, while and >> > > > target_stop_ptid is synchronous. [...] >> > > >> > > If the above code is right, I think a clarifying comment is >> > > required somewhere. It's odd that one can call >> > > agent_run_command when the inferior may or may not be stopped >> > > yet. [Or is there a bug here? - if I'm reading the gdbserver >> > > version correctly it first waits for the inferior to stop] >> > >> > It's a bug. >> > >> > (Note that the GDB side interfaces with an out-of-tree >> > agent, not GDBserver's agent. I don't know the status of >> > that agent.) >> >> Data point that target_stop should be named target_stop_async? > > Ok, can I get a summary of this thread, I'm struggling to follow it. > > a) What should the functions be called: > - target_stop_async / target_stop_wait > - target_continue_async / target_continue_no_signal > - something else? > > b) Is there a bug here I need to address? At this point I think we're still in discussion mode, there are no conclusions/agreements yet, except for the agreement to use target_continue_with_no_signal instead of target_continue_ptid. To advance the discussion, the async case is the subtle case IMO. Evidently (and I'm just going by what I see, there may be more data here) someone (*1) looked at the name "target_stop" and thought it was async (which is probably what I'd do). The function comment doesn't specify. One could argue it's sufficient to just fix the function comment, but if we're going to have a mix of similar functions, some of which are async and some sync, then IMO we should also make the difference stand out in the code where it's read. I'd be happy with a convention where all async functions ended with _async or _no_wait (the latter reads better to me), but I'm guessing I'd be happy with a different convention as well. FAOD, there is a bug, but it's not one you specifically need to address. I pointed it out because it's a data point that contributes to the discussion. (*1): I've looked at git log and blame. I'm speaking generically here because I think this is unlikely to be a one-off kind of issue. Plus I can well imagine me making a similar mistake too. Plus the bug got through code review.