| Message ID | 03006E3FC39B5A48AB9DBCCC101090A823806F4083@REDROOF2.alohasunset.com |
|---|---|
| State | New |
| Headers | show |
On 22/10/15 01:15, Mark Pizzolato wrote: > Signed-off-by: Mark Pizzolato <mark@infocomm.com> > --- > slirp/slirp.c | 2 +- > slirp/socket.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 35f819a..d18faa8 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -846,7 +846,7 @@ int slirp_remove_hostfwd(Slirp *slirp, int is_udp, struct in_addr host_addr, > getsockname(so->s, (struct sockaddr *)&addr, &addr_len) == 0 && > addr.sin_addr.s_addr == host_addr.s_addr && > addr.sin_port == port) { > - close(so->s); > + closesocket(so->s); > sofree(so); > return 0; > } > diff --git a/slirp/socket.c b/slirp/socket.c > index 37ac5cf..4a20e08 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -632,8 +632,9 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, > (listen(s,1) < 0)) { > int tmperrno = errno; /* Don't clobber the real reason we failed */ > > - close(s); > + closesocket(s); > sofree(so); > + fprintf (stderr, "Socket Error %d", tmperrno); Looks like you've left some debugging code in here? I think that should be removed. Thomas
On Tuesday, October 27, 2015 at 7:28 AM, Thomas Huth wrote: > On 22/10/15 01:15, Mark Pizzolato wrote: > > Signed-off-by: Mark Pizzolato <mark@infocomm.com> > > --- > > slirp/slirp.c | 2 +- > > slirp/socket.c | 3 ++- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/slirp/slirp.c b/slirp/slirp.c index 35f819a..d18faa8 > > 100644 > > --- a/slirp/slirp.c > > +++ b/slirp/slirp.c > > @@ -846,7 +846,7 @@ int slirp_remove_hostfwd(Slirp *slirp, int is_udp, > struct in_addr host_addr, > > getsockname(so->s, (struct sockaddr *)&addr, &addr_len) == 0 && > > addr.sin_addr.s_addr == host_addr.s_addr && > > addr.sin_port == port) { > > - close(so->s); > > + closesocket(so->s); > > sofree(so); > > return 0; > > } > > diff --git a/slirp/socket.c b/slirp/socket.c index 37ac5cf..4a20e08 > > 100644 > > --- a/slirp/socket.c > > +++ b/slirp/socket.c > > @@ -632,8 +632,9 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, > uint32_t laddr, > > (listen(s,1) < 0)) { > > int tmperrno = errno; /* Don't clobber the real reason we > failed */ > > > > - close(s); > > + closesocket(s); > > sofree(so); > > + fprintf (stderr, "Socket Error %d", tmperrno); > > Looks like you've left some debugging code in here? I think that should be > removed. Well, I thought of that, but then I followed the example in soreadbuf() in this same module where these relatively rare, maybe never, conditions happen. If a bug report came with some stderr output maybe a useful solution could be provided. In both cases, these are error path situations which the code should handle cleanly (without leaking memory or sockets). I did not actually encounter this issue during execution. I merely noticed the problem during a code review while porting to windows. I'm not stuck on this, but having these there (in both cases) seems at least consistent with the other code in the same module. A different effort could be pursued to enable these and other cases where stderr is written to in the slirp code and only produce it in debug mode. This patch is trying to minimize changes now. Other commits supplied with this one generalized the existing explicit debug output calls produced by the DEBUG_CALL, DEBUG_ARG, DEBUG_ARGS, DEBUG_MISC, DEBUG_ERROR and DPRINTF macros. If more comments come back on the rest of those patches, I'll either add an additional patch set to the existing ones to remove the explicit references to stderr and use the various debug macros as needed or I redo the whole patch set. Please suggest the best course of action. - Mark
diff --git a/slirp/slirp.c b/slirp/slirp.c index 35f819a..d18faa8 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -846,7 +846,7 @@ int slirp_remove_hostfwd(Slirp *slirp, int is_udp, struct in_addr host_addr, getsockname(so->s, (struct sockaddr *)&addr, &addr_len) == 0 && addr.sin_addr.s_addr == host_addr.s_addr && addr.sin_port == port) { - close(so->s); + closesocket(so->s); sofree(so); return 0; } diff --git a/slirp/socket.c b/slirp/socket.c index 37ac5cf..4a20e08 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -632,8 +632,9 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, (listen(s,1) < 0)) { int tmperrno = errno; /* Don't clobber the real reason we failed */ - close(s); + closesocket(s); sofree(so); + fprintf (stderr, "Socket Error %d", tmperrno); /* Restore the real errno */ #ifdef _WIN32 WSASetLastError(tmperrno);
Signed-off-by: Mark Pizzolato <mark@infocomm.com> --- slirp/slirp.c | 2 +- slirp/socket.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)