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

Pedro Alves
2020-10-26T14:00:42Z

On 10/16/20 9:51 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> This eliminates the need to specify the return type when using
> Pedro> handle_eintr. We let the compiler deduce it for us.
>
> Thanks for doing this. This is closer to how I imagined this wrapper
> being written.
>
> Pedro> +template <typename ErrorValType,
> Pedro> + typename Fun,
> Pedro> + typename... Args>
> Pedro> +inline auto
> Pedro> +handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
> Pedro> + -> decltype (f(args...))
>
> It seems to me that errval and ErrorValType are unchanging properties of
> the function being wrapped. And, normally they are -1 / int.
>
> Also is there ever a case where the return type isn't the same as
> ErrorValType?

I don't think so. But I don't think it really matters -- I look at
it as ErrorValType being the type of the value used to compare with
the wrapped function's return type.

Like, with:

ssize_t read(int fildes, void *buf, size_t nbyte);

we normally write:

ssize_t res = read (fildes, buf, nbyte);
if (res == -1)

instead of

ssize_t res = read (fildes, buf, nbyte);
if (res == (ssize_t) -1)

So ErrorValType is the type of the "-1" expression. It just
has to be convertible to the return type.

If we get rid of the ErrorValType template parameter, then we
could do instead:

template<typename Fun, typename... Args>
using return_type = decltype (std::declval<Fun> () (std::declval<Args> ()...));

template <typename Fun, typename... Args>
inline auto
handle_eintr (return_type<Fun, Args...> errval,
const Fun &f, const Args &... args)
-> decltype (f (args...))
{
decltype (f (args...)) ret;

do
{
errno = 0;
ret = f (args...);
}
while (ret == errval && errno == EINTR);

return ret;
}

That works for me too, though it looks a little more complicated,
I think.

Let me know which version you prefer.

> So maybe instead of requiring these to all be redundantly specified, the
> template could use a helper template class that specifies these things
> (defaulting to the usual), and then one would write:
>
> pid_t pid = gdb::handle_eintr<::waitpid> (...normal waitpid args);
>
> I'm not sure it's really worth implementing this, but it's closer to
> what I was picturing initially.

That thought initially crossed my mind too, but IMHO it's not worth
it. You have to write the helper template class, so it
doesn't look like a net win over writing a plain wrapper like:

int
my_waitpid (int pid, int *status, int flags)
{
return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
}

and using my_waitpid throughout. I.e., this way we also only
write the -1 once.