From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103829 invoked by alias); 23 Jan 2020 18:20:07 -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 103770 invoked by uid 89); 23 Jan 2020 18:20:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Jan 2020 18:20:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579803603; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mtafflPE2TEaEFwJnXJBCWqNwu2VV95Y8NkCPXFcwMs=; b=Y7nXfvZB7cis5JNDVtYsoUbPGnmI8nkrPeGonklGpclJE/1Ydi4pwm8LkA0PzPw/03Dtvw LYsF7tvmSNviii+LICUw1wGO26rG1OTWBvOk5YRWxxCqJvUSj+xvFEk03kzEdwEx6Miaj/ 2fUbnb7DZKtdIbL38C2zQPH+2R5leeA= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-374-Z-5fmU49NSGZFcgyOmUc6g-1; Thu, 23 Jan 2020 13:20:02 -0500 Received: by mail-wm1-f70.google.com with SMTP id f25so1536510wmb.1 for ; Thu, 23 Jan 2020 10:20:01 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id l15sm3756086wrv.39.2020.01.23.10.19.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Jan 2020 10:19:59 -0800 (PST) Subject: Re: [PATCH] gdb: fix darwin-nat.c build / adapt to multi-target To: Simon Marchi , gdb-patches@sourceware.org References: <20200123165938.32116-1-simon.marchi@efficios.com> Cc: Simon Marchi From: Pedro Alves Message-ID: Date: Thu, 23 Jan 2020 18:21:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200123165938.32116-1-simon.marchi@efficios.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-01/txt/msg00744.txt.bz2 On 1/23/20 4:59 PM, Simon Marchi wrote: > From: Simon Marchi > > The darwin-nat.c file doesn't build since the multi-target changes > (5b6d1e4f, "Multi-target support"). This patch makes it build. I have > access to a macOS vm, so I am able to build it, but I wasn't able to > successfully codesign it and try to actually debug something, so I don't > know if it works. I don't have much more time to put on this to figure > it out, so I thought I'd sent the patch anyway, as it's at least a step > in the right direction. > > The bulk of the patch is to change a bunch of functions to be methods of > the darwin_nat_target object, so that this can pass `this` to > find_inferior_ptid and other functions that now require a > process_stratum_target pointer. > > The darwin_ptrace_him function (renamed to darwin_nat_target::ptrace_him > in this patch) is passed to fork_inferior as the `init_trace_fun` > parameter. Since the method can't be passed as a plain function pointer > (we need the `this` pointer), I changed the `init_trace_fun` parameter > of fork_inferior to be a gdb::function_view, so we can pass a lambda and > capture `this`. This LGTM. Thanks for doing this. The use of function_view infork_inferior gave me pause, but it's safe since fork_inferior doesn't return until the child either exec'ed or exited -- there's no risk that the fork parent returns from the current function before the fork child accesses the function_view's closure. Note that this larger patch wasn't really necessary, since for native targets there's only ever one process_stratum_target object. So you could instead have referred to the global where you need it. But your patch is better. > diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c > index 78f972a7496d..ef5a75292fb1 100644 > --- a/gdb/nat/fork-inferior.c > +++ b/gdb/nat/fork-inferior.c > @@ -267,7 +267,8 @@ execv_argv::init_for_shell (const char *exec_file, > pid_t > fork_inferior (const char *exec_file_arg, const std::string &allargs, > char **env, void (*traceme_fun) (), > - void (*init_trace_fun) (int), void (*pre_trace_fun) (), > + gdb::function_view init_trace_fun, There should be a space in "void (int)". Thanks, Pedro Alves