[PATCH v2] gdb::handle_eintr, remove need to specify return type

Pedro Alves
2020-10-13T14:17:07Z

On 10/13/20 2:43 PM, Kamil Rytarowski wrote:

> This patch applied on trunk gives me:
>
> gmake[2]: Wejście do katalogu '/public/binutils-gdb-netbsd/build/gdbserver'
> CXX netbsd-low.o
> ../../gdbserver/netbsd-low.cc: In function ‘bool elf_64_file_p(const
> char*)’:
> ../../gdbserver/netbsd-low.cc:1152:57: error: no matching function for
> call to ‘handle_eintr(int, int (&)(const char*, int, ...), const char*&,
> int)’
> 1152 | int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
> | ^
> In file included from ../../gdbserver/netbsd-low.cc:37:
> ../../gdbserver/../gdbsupport/eintr.h:58:1: note: candidate:
> ‘template<class ErrorValType, class Fun, class ... Args> typename
> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
> const Args& ...)’
> 58 | handle_eintr (ErrorValType errval, const Fun &f, const Args &...
> args)
> | ^~~~~~~~~~~~
> ../../gdbserver/../gdbsupport/eintr.h:58:1: note: template argument
> deduction/substitution failed:
> ../../gdbserver/../gdbsupport/eintr.h: In substitution of
> ‘template<class ErrorValType, class Fun, class ... Args> typename
> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
> const Args& ...) [with ErrorValType = int; Fun = int(const char*, int,
> ...); Args = {const char*, int}]’:
> ../../gdbserver/netbsd-low.cc:1152:57: required from here
> ../../gdbserver/../gdbsupport/eintr.h:58:1: error: invalid use of
> incomplete type ‘struct gdb::return_type<int(const char*, int, ...)>’
> In file included from ../../gdbserver/../gdbsupport/poison.h:23,
> from ../../gdbserver/../gdbsupport/common-utils.h:26,
> from ../../gdbserver/../gdbsupport/common-defs.h:125,
> from ../../gdbserver/server.h:22,
> from ../../gdbserver/netbsd-low.cc:18:
> ../../gdbserver/../gdbsupport/traits.h:49:8: note: declaration of
> ‘struct gdb::return_type<int(const char*, int, ...)>’
> 49 | struct return_type;
> | ^~~~~~~~~~~

Huh, Fun is clearly a function, yet the compiler doesn't pick
the partial specialization.

I could reproduce it here, seems to be related to open being varargs.

Anyhow, we don't really need the new trait. We can just
use decltype and a trailing return. I don't know why I didn't
think of it in the first place.

Here's an updated patch. I believe this one should work for you too.

From 37d06f47a379e44a895f68a22cbb0f745da33ec5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 8 Sep 2020 17:34:41 +0100
Subject: [PATCH v2] 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 (handle_eintr): Replace Ret template parameter with
ErrorValType. Use it as type of the failure value. Deduce the
function's return type using decltype. Use lowercase for function
parameter names.
---
gdb/nat/linux-waitpid.c | 11 ++---------
gdbserver/netbsd-low.cc | 10 +++++-----
gdbsupport/eintr.h | 32 +++++++++++++++++++-------------
3 files changed, 26 insertions(+), 27 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<int> (-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<int> (-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<int> (-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<ssize_t> (-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<int> (-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..02c26e4def1 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -43,25 +43,31 @@ namespace gdb

You could wrap it by writing the wrapped form:

- ssize_t ret = gdb::handle_eintr<ssize_t> (-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. */
-
-template <typename Ret, typename Fun, typename... Args>
-inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
+ ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
+
+ ERRVAL specifies the failure value indicating that the call to the
+ F function with ARGS... arguments was possibly interrupted with a
+ signal. */
+
+template <typename ErrorValType,
+ typename Fun,
+ typename... Args>
+inline auto
+handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
+ -> decltype (f(args...))
{
- Ret ret;
+ decltype (f(args...)) 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 */

base-commit: 87a37e5e078f506fa9905b74e9238593c537fcd5
--
2.14.5