From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KLngFdiYhF+ecAAAWB0awg (envelope-from ) for ; Mon, 12 Oct 2020 13:56:40 -0400 Received: by simark.ca (Postfix, from userid 112) id 5726F1EF6F; Mon, 12 Oct 2020 13:56:40 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A29741E58D for ; Mon, 12 Oct 2020 13:56:39 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5DF06388A41F; Mon, 12 Oct 2020 17:56:39 +0000 (GMT) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by sourceware.org (Postfix) with ESMTPS id DD276388A41F for ; Mon, 12 Oct 2020 17:56:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DD276388A41F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=alves.ped@gmail.com Received: by mail-wr1-f68.google.com with SMTP id y12so14845404wrp.6 for ; Mon, 12 Oct 2020 10:56:35 -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:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=bv4u8pCesnd2aO6YFHfcYSo+ALtAOoxFTR4IsHcQwqw=; b=s6gXNAmTThafgYe4BSUSAYPt9mjc1bMnUcS86tbAO1+c3GHFQmRGNI/mK2DOPfnrIJ fKkEd3GiVbRu+8aMFLQw682vXZNeYIYem0novLqD2/1epZpBnAt/vU35cCu3H0UproBU KEeI2925R7HZoN+VzgU4BjIk1GuMeZsZHAKbTh8lMoUqgRsfUqAGyhvY8q4x6U3pwdp9 brRfq4CPpp378Tt8hbKn8sqEznQvQ6kDtf7VDMG+NhOBoGyPylq1Tikb5N+3ATIf2n6G h3j0wLLBSFkXosdIqbzqNWDZDG/rmQYL/4DESE/OO+ObUkisRRpnAAi8pErLKljwgkcF tsyw== X-Gm-Message-State: AOAM5311gckKBG9z/FKP3wxny/harI01vIqOHH1PUNNRdG+IVwewTbEY zzwCQnr/4ZaSTd+nUt8TURoFhGpXiwWp6ggX X-Google-Smtp-Source: ABdhPJxBi2jzCBSktFrfNeQRBnRZeyN9ajMoyTXfErdsbGWVfQiRTjMIL4L6TBEK2CywtjYc8Hjneg== X-Received: by 2002:adf:e292:: with SMTP id v18mr30795051wri.256.1602525394907; Mon, 12 Oct 2020 10:56:34 -0700 (PDT) Received: from ?IPv6:2001:8a0:f91e:6d00:c80a:ea25:47ef:5f73? ([2001:8a0:f91e:6d00:c80a:ea25:47ef:5f73]) by smtp.gmail.com with ESMTPSA id c21sm23869793wme.36.2020.10.12.10.56.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 12 Oct 2020 10:56:33 -0700 (PDT) Subject: [PATCH] gdb::handle_eintr, remove need to specify return type (Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls) To: Kamil Rytarowski , Simon Marchi , gdb-patches@sourceware.org References: <20200904002905.13616-1-n54@gmx.com> <20200904002905.13616-2-n54@gmx.com> From: Pedro Alves Message-ID: <75070000-03a2-eeed-4f72-e29f199291af@palves.net> Date: Mon, 12 Oct 2020 18:56:32 +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: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , Cc: tom@tromey.com Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi! On 9/7/20 3:59 PM, Kamil Rytarowski wrote: > On 07.09.2020 16:06, Simon Marchi wrote: >> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote: >>> +#ifndef GDBSUPPORT_EINTR_H >>> +#define GDBSUPPORT_EINTR_H >>> + >>> +#include >>> + >>> +namespace gdb >>> +{ >>> +template >>> +inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A) >>> +{ >>> + Ret ret; >>> + do >>> + { >>> + errno = 0; >>> + ret = F (A...); >>> + } >>> + while (ret == R && errno == EINTR); >>> + return ret; >>> +} >>> +} >> >> Can you document this function a little bit, including a usage example? I tried >> to apply it in gdb/linux-nat.c, function async_file_mark, but I'm not sure I'm >> doing it correctly. In particular, what should I pass as the `R` parameter? Does >> that make sense? >> > > I'm going to add an example and some documentation. > >> gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1); >> > > gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1); Going through my email backlog I noticed this. This requirement to explicitly specify the return type at each caller seems unnecessary. We can make the compiler deduce it for us based on the return type of the wrapped function. I gave it a shot -- do you see an issue with the change below? I adjusted the NetBSD files, but I don't have an easy way to confirm they still build OK, though I expect they do. I converted Linux's my_waitpid to use the adjusted handle_eintr, and it worked OK. >From 0c2cf066f3f34a9aa3de7aab7bc2bca80b557b45 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 8 Sep 2020 17:34:41 +0100 Subject: [PATCH] gdb::handle_eintr, remove need to specify return type This eliminates the need to specify the return type when using handle_eintr. We let the compiler deduce it for us. Also, use lowercase for function parameter names. Uppercase should only be used on template parameters. gdb/ChangeLog: * nat/linux-waitpid.c: Include "gdbsupport/eintr.h". (my_waitpid): Use gdb::handle_eintr. gdbserver/ChangeLog: * netbsd-low.cc (netbsd_waitpid, netbsd_process_target::kill) (netbsd_qxfer_libraries_svr4): Use gdb::handle_eintr without explicit type. gdbsupport/ChangeLog: * eintr.h: Include "gdbsupport/traits.h". (handle_eintr): Replace Ret template parameter with ErrorValType. Use it as type of the failure value. Deduce the function's return type using gdb::return_type. Use lowercase for function parameter names. * traits.h (struct return_type): New. --- gdb/nat/linux-waitpid.c | 11 ++--------- gdbserver/netbsd-low.cc | 10 +++++----- gdbsupport/eintr.h | 29 ++++++++++++++++++----------- gdbsupport/traits.h | 11 +++++++++++ 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c index f50e0c7bcff..d066239220a 100644 --- a/gdb/nat/linux-waitpid.c +++ b/gdb/nat/linux-waitpid.c @@ -22,6 +22,7 @@ #include "linux-nat.h" #include "linux-waitpid.h" #include "gdbsupport/gdb_wait.h" +#include "gdbsupport/eintr.h" /* Convert wait status STATUS to a string. Used for printing debug messages only. */ @@ -54,13 +55,5 @@ status_to_str (int status) int my_waitpid (int pid, int *status, int flags) { - int ret; - - do - { - ret = waitpid (pid, status, flags); - } - while (ret == -1 && errno == EINTR); - - return ret; + return gdb::handle_eintr (-1, ::waitpid, pid, status, flags); } diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc index 30028d3a384..519614d0473 100644 --- a/gdbserver/netbsd-low.cc +++ b/gdbserver/netbsd-low.cc @@ -245,7 +245,7 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0; pid_t pid - = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options); + = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options); if (pid == -1) perror_with_name (_("Child process unexpectedly missing")); @@ -456,7 +456,7 @@ netbsd_process_target::kill (process_info *process) return -1; int status; - if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1) + if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1) return -1; mourn (process); return 0; @@ -1149,15 +1149,15 @@ netbsd_qxfer_libraries_svr4 (const pid_t pid, const char *annex, static bool elf_64_file_p (const char *file) { - int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY); + int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY); if (fd < 0) perror_with_name (("open")); Elf64_Ehdr header; - ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header)); + ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header)); if (ret == -1) perror_with_name (("read")); - gdb::handle_eintr (-1, ::close, fd); + gdb::handle_eintr (-1, ::close, fd); if (ret != sizeof (header)) error ("Cannot read ELF file header: %s", file); diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h index 64ff5940b75..ab8310e561c 100644 --- a/gdbsupport/eintr.h +++ b/gdbsupport/eintr.h @@ -22,6 +22,8 @@ #include +#include "gdbsupport/traits.h" + namespace gdb { /* Repeat a system call interrupted with a signal. @@ -43,25 +45,30 @@ namespace gdb You could wrap it by writing the wrapped form: - ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1); + ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1); - The RET typename specifies the return type of the wrapped system call, which - is typically int or ssize_t. The R argument specifies the failure value - indicating the interrupted syscall when calling the F function with - the A... arguments. */ + ERRVAL specifies the failure value indicating that the call to the + F function with ARGS... arguments was possibly interrupted with a + signal. */ -template -inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A) +template +inline typename return_type::type +handle_eintr (ErrorValType errval, const Fun &f, const Args &... args) { - Ret ret; + typename return_type::type ret; + do { errno = 0; - ret = F (A...); + ret = f (args...); } - while (ret == R && errno == EINTR); + while (ret == errval && errno == EINTR); + return ret; } -} + +} /* namespace gdb */ #endif /* GDBSUPPORT_EINTR_H */ diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h index 93b609ac109..f161ca00fa6 100644 --- a/gdbsupport/traits.h +++ b/gdbsupport/traits.h @@ -43,6 +43,17 @@ namespace gdb { +/* Use partial specialization to get access a function's return + type. */ +template +struct return_type; + +template +struct return_type +{ + typedef Res type; +}; + /* Pre C++14-safe (CWG 1558) version of C++17's std::void_t. See . */ base-commit: 87a37e5e078f506fa9905b74e9238593c537fcd5 -- 2.14.5