Re: [PATCH v2 1/5] Import "glob" and "getcwd" modules from gnulib

[Pedro Alves <palves@redhat.com>]

On 09/20/2017 06:17 PM, Sergio Durigan Junior wrote:
> On Wednesday, September 20 2017, Pedro Alves wrote:

>> --- a/gdb/ser-tcp.c
>> +++ b/gdb/ser-tcp.c
>> @@ -42,7 +42,6 @@
>>  #ifndef ETIMEDOUT
>>  #define ETIMEDOUT WSAETIMEDOUT
>>  #endif
>> -#define close(fd) closesocket (fd)
>>
>> Are you sure that the gnulib code that makes close work
>> for sockets is enabled?  I guess it will if WINDOWS_SOCKETS
>> is defined, but it wasn't obvious to me whether it'll end
>> up defined with the current set of modules.
> 
> One of the dependencies of "getcwd" is the "close" module, and whenever
> I tried to compile this patch with mingw it would fail because of this
> re-definition of close made by ser-tcp.c.  My first approach was to
> #undef close before ser-tcp.c redefined it, but then I decided to just
> use "close" from gnulib.  I didn't know about this WINDOWS_SOCKETS
> requirement; maybe we can define it before the inclusion of <stdlib.h>?
> Or maybe choose the safe side and let ser-tcp.c redefine close as
> needed.

I don't know much about WINDOWS_SOCKETS either.  I just looked
at gnulib/lib/close.c, and found:

~~~
/* Override close() to call into other gnulib modules.  */

int
rpl_close (int fd)
{
#if WINDOWS_SOCKETS
  int retval = execute_all_close_hooks (close_nothrow, fd);
#else
  int retval = close_nothrow (fd);
#endif

#if REPLACE_FCHDIR
  if (retval >= 0)
    _gl_unregister_fd (fd);
#endif

  return retval;
}
~~~


and then figured out that there's a close_fd_maybe_socket
close hook implemented in lib/sockets.c.

static int
close_fd_maybe_socket (const struct fd_hook *remaining_list,
                       gl_close_fn primary,
                       int fd)
{

This is all wrapped in #ifdef WINDOWS_SOCKETS, hence the question.

It should be easy for you to determine whether WINDOWS_SOCKETS
is defined in your mingw build, and thus whether all this code
is part of the build or not.

> 
>> Boy, these modules sure bring in a lot of stuff.
> 
> Yeah, "getcwd" did.
> 
>> I noticed that this is bringing in the strerror module, which was
>> problematic in the past on mingw [1].  Could you please try
>> cross building gdb for mingw (should be easy since Fedora has
>> cross compilers handy), to confirm that it's alright now?
>> It it works, we could get rid of safe_strerror in a follow up
>> patch.
>>
>> [1] https://sourceware.org/ml/gdb-patches/2013-11/msg00597.html
>> I don't recall whether that's been fixed meanwhile.
> 
> Before submitting the patch I had compiled it for mingw, and everything
> worked OK.  I did it again just in case, and it's still working.  So I
> believe the issue is fixed.

Alright, that's great.

Thanks,
Pedro Alves