Re: [PATCH 4/4] gnulib: import select

[Mike Frysinger <vapier@gentoo.org>]

On 10 Jun 2021 00:30, Pedro Alves wrote:
> On 2021-06-09 7:32 p.m., Mike Frysinger wrote:
> > On 09 Jun 2021 19:12, Pedro Alves wrote:
> >> On 2021-06-09 5:52 p.m., Mike Frysinger wrote:
> >>> On 09 Jun 2021 13:27, Pedro Alves wrote:
> >>>> I've always feared that gnulib's select module would conflict with that,
> >>>> but I don't know for sure.  In theory, we should be able to replace all
> >>>> that code by using gnulib's select instead, though I'm not sure gnulib
> >>>> handles some particulars like Ctrl-C interruption.
> >>>
> >>> why do you think it would conflict ?  the file doesn't seem to use select()
> >>> at all, and ser-mingw is always enabled for mingw targets.  seems like the
> >>> func would just be ignored.
> >>
> >> gnulib also sometimes replaces headers with their own incompatible
> >> definitions.
> > 
> > good point, but i'm not sure how that matters to ser-mingw specifically.
> > it isn't including any select.h related headers, just Windows-specific ones.
> 
> I meant to also point at mingw-hdep.c, which is where gdb_select implementation lives.
> That of course, includes sys/select.h via gdb_select.h.
> I misremembered that gdb_select itself was in ser-mingw.c too.  mingw's version
> of gdb_select is built on top of the ser-mingw.c and struct serial, but lives
> in mingw-hdep.c.
> 
> Also, the sockets module was pulled in as well as dependency, and I suppose that hides the SOCKET vs
> int, close vs closesocket, _get_osfhandle, read/write vs send/recv etc. WinSock2 particularities,
> and don't know whether that can cause issues in the code that we have that assumes native Windows
> sockets on mingw, which you'll find a lot of if you look for USE_WIN32API, particularly in
> gdbserver and gdbreplay.
> 
> If it does, it may not be noticeable with a build-only check.
> 
> Even if it doesn't, there's now a lot of USE_WIN32API code in gdb, gdbsupport
> and gdbserver that is now ripe for cleansing.  In addition to close vs closesocket, read vs send
> etc I mentioned above, there should be no need anymore to include the windows/winsock headers to
> get access to sockets things, we should call gl_sockets_startup instead of WSAStartup, etc.

hmm, so how gnulib is integrated leads to "interesting" behavior.  we don't
run configure against it, we only compile+link against it.  so when gdb's
configure probes sys/select.h, it doesn't find it.  and then the mingw files
(and files it includes) that are protected by HAVE_SYS_SELECT_H are not used.

on the sim side, i dropped the configure tests entirely and changed them to
assume the headers/funcs exist.

> > i haven't gone through that list (and indirect includes) explicitly.  but i
> > would expect the build to at least warn or fail if that were the case.
> > 
> > did you have any specific concerns here ?  or just aiming for due diligence
> > with the testsuite to assuage general unease ?
> 
> The latter, but I wasn't thinking of a testsuite run, which is notoriously difficult
> on Windows.  More like a smoke test, run GDB, to make sure you can still debug something.
> 
> Also, a quick check to make sure remote debugging against a Windows GDBserver built
> from master still works would be good -- gdbserver also uses gnulib, and uses select
> (not gdb_select) and sockets.
> 
> Interestingly, this module import may have put us closer to removing the Windows
> gdbserver limitation that it only works with tcp/ip socket connections, particularly
> interesting being pipes, for "target remote | gdbserver" support.

i can provide gdb.exe's easily, but i imagine most of us can do that now too.
i think Debian includes a bunch of mingw toolchains.
-mike