Re: [PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[Tom de Vries <tdevries@suse.de>]

On 11/29/24 17:15, Simon Marchi wrote:
> 
> 
> On 2024-11-22 11:46, Tom de Vries wrote:
>> On 11/5/24 13:07, Tom de Vries wrote:
>>> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>>
>>>> Tom> +namespace gdb {
>>>> Tom> +
>>>> Tom> +extern "C" {
>>>>
>>>> I didn't even know this was valid ... but is it really needed?
>>>> It seems wrong.
>>>>
>>>
>>> It seemed to work:
>>> ...
>>> $ cat test.c
>>> namespace gdb {
>>>
>>>     extern "C" {
>>>       int foo (void) { return 1; };
>>>     }
>>> }
>>>
>>> $ cat test2.c
>>> namespace gdb {
>>>     extern "C" {
>>>       int foo (void);
>>>     }
>>> }
>>>
>>> int
>>> main (void)
>>> {
>>>     return gdb::foo ();
>>> }
>>> $ g++ test.c test2.c
>>> ...
>>> but the linkage symbol is plain foo:
>>> ...
>>> $ nm ./a.out  | grep foo
>>> 00000000004004c7 T foo
>>> ...
>>>
>>> So, you're right, this is wrong.
>>>
>>> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, similar to gdb_select, but thinking about it now I slightly prefer gdb::waitpid.  I suppose it doesn't matter that much.
>>>
>>
>> When looking at this again today, it bugged me enough rewrite v2 in the gdb::waitpid style.  Committed after retesting.
> 
> When building for mingw-w64, I see this, which seems related:
> 
>        CXX    cli/cli-cmds.o
>      In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:77:35: error: ‘::waitpid’ has not been declared; did you mean ‘gdb::waitpid’?
>         77 |   return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
>            |                                   ^~~~~~~
>            |                                   gdb::waitpid
>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>         75 | waitpid (pid_t pid, int *wstatus, int options)
>            | ^~~~~~~
> 
> Note that to get to it, you have to step past this other build error in
> gdbserver (by doing `make all-gdb` for instance):
> 
>    https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17

Hi Simon,

thanks for reporting this.

I don't have a windows setup unfortunately, so I have no way of 
reproducing this, and at this point I have no idea why waitpid is not 
declared.

Thanks,
- Tom