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

[Pedro Alves <pedro@palves.net>]

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.