This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] Add IPv6 support for outgoing remote TCP connections
- From: Paul Fertser <fercerpav at gmail dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: Eli Zaretskii <eliz at gnu dot org>, gdb-patches at sourceware dot org, ktietz at redhat dot com
- Date: Wed, 12 Feb 2014 21:32:05 +0400
- Subject: Re: [PATCH v3] Add IPv6 support for outgoing remote TCP connections
- Authentication-results: sourceware.org; auth=none
- References: <20140210195758 dot GA16956 at host2 dot jankratochvil dot net> <1392148089-18253-1-git-send-email-fercerpav at gmail dot com> <20140212165318 dot GA8969 at host2 dot jankratochvil dot net>
Hi Jan,
Thank you for the review!
On Wed, Feb 12, 2014 at 05:53:18PM +0100, Jan Kratochvil wrote:
> > +AC_SEARCH_LIBS(getaddrinfo, [socket network net], [], [], [-lnsl])
>
> Where is it stated in gnulib? I could not find it.
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=m4/getaddrinfo.m4;h=2e66584865e9b45c201219071a5abc454ef43937;hb=HEAD#l21
-lnsl is needed per Oracle getaddrinfo man page.
> > #ifdef USE_WIN32API
> > #include <winsock2.h>
> > +#include <ws2tcpip.h>
>
> Put there AC_CHECK_HEADERS for it unless someone confirms all MS-Windows hosts
> really have that include file. I have no idea.
Hm, ok, I'll try to come up with something justifiable here.
> > + tmp = getaddrinfo (hostname, port_str, &hints, &result);
> > + if (tmp)
> > {
> > fprintf_unfiltered (gdb_stderr, "%s: unknown host\n", hostname);
>
> It should use gai_strerror(). I understand you wrote
> # In this patch I was trying to change current behaviour as little as
> # possible.
> It can be a separate patch 2/2 or even in this one.
Ok.
> > + for (rp = result; ; rp = rp->ai_next ? rp->ai_next : result)
> > + {
> > + scb->fd = gdb_socket_cloexec (rp->ai_family, rp->ai_socktype,
> > + rp->ai_protocol);
> >
> > - if (scb->fd == -1)
> > - return -1;
> > + if (scb->fd == -1)
> > + continue;
>
> Neither 'return -1' nor 'continue' are right here. It should not lock-up if
> none of the sockets can be created, it should return so that the error gets
> reported.
Indeed, thank you for spotting this! So can it be fprintf_unfiltered +
return -1 here?
I'm still puzzled about native windows behaviour; at least when run
with wine this code I proposed performs differently compared to
GNU/Linux, but I think it is the case with the unmodified code too.
--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com