Re: [PATCH] gdb::handle_eintr, remove need to specify return type (Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls)

[Kamil Rytarowski <kamil@netbsd.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3613 bytes --]

On 12.10.2020 19:56, Pedro Alves wrote:
> Hi!
> 

Hi Pedro,

> 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 <cerrno>
>>>> +
>>>> +namespace gdb
>>>> +{
>>>> +template <typename Ret, typename Fun, typename... Args>
>>>> +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<ssize_t> (-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?
> 

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;
      |        ^~~~~~~~~~~
gmake[2]: *** [Makefile:551: netbsd-low.o] Błąd 1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]