The default bug view has changed. See this FAQ.

PR_Connect() fails on panther beta

RESOLVED FIXED in 4.4.1

Status

NSPR
NSPR
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Brian Ryner (not reading), Assigned: Wan-Teh Chang)

Tracking

other
4.4.1
PowerPC
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
As wtc and I discussed, PR_Connect() fails on the Panther beta because the OS
connect() returns EINVAL.  The problem appears to be that the IPV6_V6ONLY socket
option is enabled by default for AF_INET6 sockets.
(Reporter)

Comment 1

14 years ago
Created attachment 126972 [details] [diff] [review]
patch from wtc

This is a patch wtc sent to me.
(Reporter)

Comment 2

14 years ago
Comment on attachment 126972 [details] [diff] [review]
patch from wtc

r=bryner.  One minor thing, I noticed that the prototype for setsockopt has the
4th argument as a void*, not char*.

wtc, can you check this in on the trunk and client branch, if you think it's
ok?
Attachment #126972 - Flags: review+

Comment 3

14 years ago
Have we filed a bug with Apple on that option being on by default?
(Reporter)

Comment 4

14 years ago
Radar bug 3314011.
(Assignee)

Comment 5

14 years ago
Created attachment 127274 [details] [diff] [review]
patch from wtc, v2

This patch performs a test during NSPR initialization
to find out whether we need to turn off the IPV6_V6ONLY
option.  This way we can save the setsockopt calls on
the Mac OS X releases that turn off the IPV6_V6ONLY option
by default per RFC 3493.

Please review and test this patch.
Attachment #126972 - Attachment is obsolete: true
(Assignee)

Comment 6

14 years ago
Comment on attachment 127274 [details] [diff] [review]
patch from wtc, v2

The only concern I have about this patch is that
the use of IPV6_V6ONLY may break the build on Mac
OS X 10.1.  I don't know if that macro is defined
on 10.1.  Anyone knows?
Attachment #127274 - Flags: superreview?(sfraser)
Attachment #127274 - Flags: review?(bryner)
(Reporter)

Comment 7

14 years ago
Comment on attachment 127274 [details] [diff] [review]
patch from wtc, v2

Could we possibly test this just once, instead of every time we create a
socket?
(Reporter)

Comment 8

14 years ago
Never mind my last comment, I saw the socket() nearby and figured this was
during socket creation.
(Reporter)

Comment 9

14 years ago
Comment on attachment 127274 [details] [diff] [review]
patch from wtc, v2

r=bryner.  I tested this and it works fine.  (The code indentation of the code
in _pr_test_ipv6_socket looks a little off though).
Attachment #127274 - Flags: review?(bryner) → review+
(Assignee)

Comment 10

14 years ago
Created attachment 127321 [details] [diff] [review]
Patch that was checked in

This is the same as patch v2 except that I fixed the
indentation problem and defined what "Panther" is in
the comment.
(Assignee)

Comment 11

14 years ago
Comment on attachment 127321 [details] [diff] [review]
Patch that was checked in

This patch has been checked into the NSPR tip (NSPR 4.4.1)
and the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.5a).
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → 4.4.1

Comment 12

14 years ago
As suspected the patch causes the build to fail on OS X 10.1.5.

From my build attempt:
...
ptio.c: In function `pt_Connect':
ptio.c:1530: warning: unused variable `addrp'
ptio.c: In function `pt_Bind':
ptio.c:1715: warning: unused variable `addrp'
ptio.c: In function `pt_SendTo':
ptio.c:1965: warning: unused variable `addrp'
ptio.c: In function `_pr_test_ipv6_socket':
ptio.c:3397: `IPV6_V6ONLY' undeclared (first use in this function)
ptio.c:3397: (Each undeclared identifier is reported only once
ptio.c:3397: for each function it appears in.)
ptio.c: In function `PR_Socket':
ptio.c:3459: `IPV6_V6ONLY' undeclared (first use in this function)
ptio.c: In function `_pr_poll_with_poll':
ptio.c:3723: warning: `start' might be used uninitialized in this function
ptio.c: In function `PR_Select':
ptio.c:4791: warning: `start' might be used uninitialized in this function
make[7]: *** [ptio.o] Error 1
make[7]: Leaving directory `/usr/local/cvsrep/moz/mozilla/nsprpub/pr/src/pthreads'
...

Comment 13

14 years ago
Created attachment 127740 [details] [diff] [review]
patch by df for 10.1

This patch gets past the problem on my machine. (I know nothing of C/C++ so
it's probably horribly broken. Please forgive - thought it might still be
helpful.) It seems to do the trick, just adds a few checks to see if
defined(IPV6_V6ONLY). My build attempts still fails, though much later and in a
completely unrelated area.
(Assignee)

Comment 14

14 years ago
Created attachment 127742 [details] [diff] [review]
Patch to define IPV6_V6ONLY on 10.1

Could you test this patch on Mac OS X 10.1?  Thanks.
Attachment #127740 - Attachment is obsolete: true

Comment 15

14 years ago
This patch works for me on 10.1. Thanks!
(Assignee)

Comment 16

14 years ago
Created attachment 127824 [details] [diff] [review]
Second patch that was checked in

This is the second patch that was checked in.
I also fixed the build problem for Mac OS X
deployment target 10.2 or higher.
(Assignee)

Comment 17

14 years ago
Both patches have been checked in.

Mozilla 1.5a only has the first patch.  So it
won't build on Mac OS X 10.1 or for Mac OS X
deployment target 10.2 or higher.

Mozilla 1.5b will have both patches.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Updated

14 years ago
Attachment #127274 - Flags: superreview?(sfraser)
You need to log in before you can comment on or make changes to this bug.