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
> > ---
> > 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
