This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- From: Doug Evans <dje at google dot com>
- To: Gary Benson <gbenson at redhat dot com>
- Cc: Pedro Alves <palves at redhat dot com>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Mon, 15 Sep 2014 09:00:40 -0700
- Subject: Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Authentication-results: sourceware.org; auth=none
- References: <21520 dot 36381 dot 756875 dot 963606 at ruffy2 dot mtv dot corp dot google dot com> <20140911102659 dot GA17472 at blade dot nx> <5412DEB5 dot 6020706 at redhat dot com> <21523 dot 9502 dot 168492 dot 803068 at ruffy2 dot mtv dot corp dot google dot com> <54132B55 dot 9000108 at redhat dot com> <21523 dot 12189 dot 134570 dot 770432 at ruffy2 dot mtv dot corp dot google dot com> <5413305B dot 6020402 at redhat dot com> <21523 dot 13993 dot 986533 dot 615240 at ruffy2 dot mtv dot corp dot google dot com> <54133939 dot 70801 at redhat dot com> <CADPb22RsGv_Do1SztOK4Bse99e5yL_hmnrOHCU8OhNGkFyxGDQ at mail dot gmail dot com> <20140915100736 dot GA13503 at blade dot nx>
On Mon, Sep 15, 2014 at 3:07 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> On Fri, Sep 12, 2014 at 11:19 AM, Pedro Alves <palves@redhat.com> 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.
- References:
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid
- Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid