From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 7B7AD38930F4 for ; Sat, 20 Jun 2020 19:26:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7B7AD38930F4 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-24-mellIlD4NyiL9Z4W-jpVZA-1; Sat, 20 Jun 2020 15:26:00 -0400 X-MC-Unique: mellIlD4NyiL9Z4W-jpVZA-1 Received: by mail-wr1-f70.google.com with SMTP id z3so599555wrr.7 for ; Sat, 20 Jun 2020 12:25:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=MkmvIThJgnyIdmxV4oMiMJsx+82ug+JGmqa/VcMIUXU=; b=uSesaoqY2xDhnsh5eLbqIey7d2DSANnUVAXlYySh0td+LFKdb/4JZfDlSM2Be0kEK5 Msbh6vtYittiGEBCfrHvLDv2DwI/2/VAtqDCvmAkZlVhgwcgm2+hKmqfCrI4TTq7nWbD YkpJB218RznESLYVa+KnwGMgRDKp9+2+VM/+ozV1sPZ3TkLj69edQziKAqJXekcxXUtZ mpZIdQ2z4qtrc1Iuwt9mgZRAFOwe34oWS7Ir0ANWrkLQXokzMgx0fk5HUhXGk85cZXpd LrVkOKvEcf72Ord7Yhq8lu8rFaaa0hszEFiDtgEDrixJOGh9qlH4WIcu08j0f9+CbD6F rX7g== X-Gm-Message-State: AOAM532d5zQhxlQL9GuTn7c4ZQkIKZ/MJLdRtt3sm66htwv3YdNMl9z0 8fv5AxR2bs7/3h872dO29KfZrk4ZBeXE0FavOC1rwKnbfqj9dZ5m+EQS3CJ1kmeAGY3PhVUdSg6 JsTSTLbfY+TbMRSNtw6C1kA== X-Received: by 2002:adf:c3c7:: with SMTP id d7mr10184615wrg.51.1592681158758; Sat, 20 Jun 2020 12:25:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzuSXYqs4Z6NwWDOoMotPD/MmxU6to3qGDZOcEDsG+CXKUdJLRFVennnsDqnMzfueuIo/iMnQ== X-Received: by 2002:adf:c3c7:: with SMTP id d7mr10184603wrg.51.1592681158519; Sat, 20 Jun 2020 12:25:58 -0700 (PDT) Received: from ?IPv6:2001:8a0:f922:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f922:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id 104sm11388957wrl.25.2020.06.20.12.25.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 20 Jun 2020 12:25:57 -0700 (PDT) Subject: Re: [PATCH] gdbserver/linux-low: use std::list to store pending signals To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: <1592568732-3668-1-git-send-email-tankut.baris.aktemur@intel.com> From: Pedro Alves Message-ID: Date: Sat, 20 Jun 2020 20:25:54 +0100 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: <1592568732-3668-1-git-send-email-tankut.baris.aktemur@intel.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Jun 2020 19:26:03 -0000 LGTM with a few nits below. On 6/19/20 1:12 PM, Tankut Baris Aktemur via Gdb-patches wrote: > @@ -2144,32 +2132,24 @@ enqueue_one_deferred_signal (struct lwp_info *lwp, int *wstat) > twice) */ > if (WSTOPSIG (*wstat) < __SIGRTMIN) > { > - struct pending_signals *sig; > - > - for (sig = lwp->pending_signals_to_report; > - sig != NULL; > - sig = sig->prev) > + for (auto &sig : lwp->pending_signals_to_report) const auto ? > - p_sig = XCNEW (struct pending_signals); > - p_sig->prev = lwp->pending_signals_to_report; > - p_sig->signal = WSTOPSIG (*wstat); > + lwp->pending_signals_to_report.emplace_back (WSTOPSIG (*wstat)); > > ptrace (PTRACE_GETSIGINFO, lwpid_of (thread), (PTRACE_TYPE_ARG3) 0, > - &p_sig->info); > - > - lwp->pending_signals_to_report = p_sig; > + &(lwp->pending_signals_to_report.back ().info)); Redundant parens. > } > > /* Dequeue one signal from the "signals to report later when out of > @@ -2180,20 +2160,16 @@ dequeue_one_deferred_signal (struct lwp_info *lwp, int *wstat) > { > struct thread_info *thread = get_lwp_thread (lwp); > > - if (lwp->pending_signals_to_report != NULL) > + if (!lwp->pending_signals_to_report.empty ()) > { > - struct pending_signals **p_sig; > + const pending_signal &p_sig = lwp->pending_signals_to_report.front (); > > - p_sig = &lwp->pending_signals_to_report; > - while ((*p_sig)->prev != NULL) > - p_sig = &(*p_sig)->prev; > - > - *wstat = W_STOPCODE ((*p_sig)->signal); > - if ((*p_sig)->info.si_signo != 0) > + *wstat = W_STOPCODE (p_sig.signal); > + if (p_sig.info.si_signo != 0) > ptrace (PTRACE_SETSIGINFO, lwpid_of (thread), (PTRACE_TYPE_ARG3) 0, > - &(*p_sig)->info); > - free (*p_sig); > - *p_sig = NULL; > + &(p_sig.info)); Redundant parens. > + > + lwp->pending_signals_to_report.pop_front (); > > if (debug_threads) > debug_printf ("Reporting deferred signal %d for LWP %ld.\n", > @@ -2201,13 +2177,9 @@ dequeue_one_deferred_signal (struct lwp_info *lwp, int *wstat) > > if (debug_threads) > { > - struct pending_signals *sig; > - > - for (sig = lwp->pending_signals_to_report; > - sig != NULL; > - sig = sig->prev) > + for (auto &sig : lwp->pending_signals_to_report) const ? > debug_printf (" Still queued %d\n", > - sig->signal); > + sig.signal); > > debug_printf (" (no more queued signals)\n"); > } > @@ -4050,15 +4022,9 @@ linux_process_target::stop_all_lwps (int suspend, lwp_info *except) > static void > enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info) > { > - struct pending_signals *p_sig = XNEW (struct pending_signals); > - > - p_sig->prev = lwp->pending_signals; > - p_sig->signal = signal; > - if (info == NULL) > - memset (&p_sig->info, 0, sizeof (siginfo_t)); > - else > - memcpy (&p_sig->info, info, sizeof (siginfo_t)); > - lwp->pending_signals = p_sig; > + lwp->pending_signals.emplace_back (signal); > + if (info != nullptr) > + lwp->pending_signals.back ().info = *info; I noticed that the pending_signal ctor always initializes the info field. Seems wasteful when we're practically always going to overwrite it. How about not initializing it in the ctor, and then here do the memset if info == NULL, as before? Thanks, Pedro Alves