17184 - PR_Bind should not set the SO_REUSEADDR socket option.

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

Right now PR_Bind sets the SO_REUSEADDR socket option
before calling bind.  There is no way a caller can
override this behavior.

PR_Bind should not set the SO_REUSEADDR socket option.
Those who need SO_REUSEADDR should call PR_SetSocketOption
with PR_SockOpt_Reuseaddr to set it.

Comment 1

[Wan-Teh Chang]

I am attaching a patch file.  Please review it.
Comments on the patch:
1. The HAVE_SOCKET_REUSEADDR macro is now obsolete
   and removed from the _xxxos.h files.
2. The setsockopt(SO_REUSEADDR) call is removed from
   SocketBind and pt_Bind in prsocket.c and ptio.c,
   respectively.
3. Some of our tests depend on the SO_REUSEADDR
   socket option.  I fixed accept.c so that it tries
   binding to a different port if a port is in use.
   For the other tests I took the easy way out and
   just added a PR_SetSocketOption call before PR_Bind.

With this patch applied, all tests passed on Solaris.

Comment 2

[Wan-Teh Chang]

Attachment: Proposed patch.

Comment 3

[Wan-Teh Chang]

Judson, PR_Bind automatically sets the SO_REUSEADDR
socket option before calling bind.  We are going to
remove this setsockopt call from PR_Bind.  I'm wondering
if Mozilla depends on the SO_REUSEADDR option.

Searching in the SeaMonkey source I found that PR_Bind
is called in three files:
mozilla/extensions/irc/libbs/bsserver.c
mozilla/network/main/mkconect.c
mozilla/network/protocol/ftp/mkftp.c

I don't know who owns extensions/irc.
The PR_Bind call in bsserver.c seems wrong
because the 'inet.port' field of the address
is uninitialized.  So I'm not going to worry
about this file.

The PR_Bind calls in mkconect.c don't
need SO_REUSEADDR because they bind to
INADDR_ANY and port 0.  I don't know if
mkftp.c depends on SO_REUSEADDR.  If it
does, you can add the following code before
the PR_Bind call:
    PRFileDesc *listen_sock;
    PRSocketOptionData opt;

    opt.option = PR_SockOpt_Reuseaddr;
    opt.value.reuse_addr = PR_TRUE;
    PR_SetSocketOption(listen_sock, &opt); /* error handling is omitted */

Please let me know when you've taken care of this.

Comment 4

[Judson Valeski]

I'm not sure who owns the extensions stuff. the network stuff is dead code.
mozilla networking is done in netwerk (with an 'e').

Comment 5

[srinivas]

The patch looks good.

Comment 6

[Wan-Teh Chang]

I checked in the patch to the main trunk.

The patch is also checked into the internal repository (/m/src).