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: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Paul Fertser <fercerpav at gmail 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:10:02 +0100
- 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> <20140212173205 dot GC26683 at home dot lan>
Hi Paul,
On Wed, 12 Feb 2014 18:32:05 +0100, Paul Fertser wrote:
> 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.
Thanks, fine with it now.
> > > + 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?
Currently I think it should do:
any_socket_opened = 0;
for (rp = result; ; rp = rp->ai_next ? rp->ai_next : result) {
scb->fd = gdb_socket_cloexec (...);
if (scb->fd == -1) {
if (rp->ai_next == NULL && (!any_socket_opened || !tcp_auto_retry))
return -1;
continue;
}
any_socket_opened = 1;
[...]
}
It is not perfect for the case
if (rp->ai_next == NULL && any_socket_opened && !tcp_auto_retry)
as errno will be returned from the failed gdb_socket_cloexec() while it would
be more appropriate to return errno from formerly failed connect(). But I do
not think it matters too much, sure it can be also fixed.
But that "return -1" part also depends on the requested gai_strerror()
implementation - currently the caller always calls perror_with_name() which
will be no longer right with gai_strerror().
I also think that it does not behave correctly now with:
set tcp auto-retry off
It will abort on first IPv6 "connection refused" despite IMO it should still
try even the IPv4 entry (although only once due to "set tcp auto-retry off").
> 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.
I do not think the Wine run is too important. I was trying to use Wine to
test the MinGW port before but I do not find the Wine run of MinGW GDB usable
for too may other reasons. After the IPv6 patch gets finalized we can ask
someone with native MS-Windows access to verify the behavior. Nice you have
found this difference.
Thanks,
Jan