Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

small patch for unix/socketaddr.c #4810

Closed
vicuna opened this issue May 29, 2009 · 5 comments
Closed

small patch for unix/socketaddr.c #4810

vicuna opened this issue May 29, 2009 · 5 comments
Assignees
Labels

Comments

@vicuna
Copy link

vicuna commented May 29, 2009

Original bug ID: 4810
Reporter: jaap
Assigned to: @xavierleroy
Status: closed (set by @xavierleroy on 2009-10-18T08:37:02Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 3.11.1+rc0
Fixed in version: 3.11.2+dev
Category: ~DO NOT USE (was: OCaml general)

Bug description

Under NetBSD, the Unix.sockaddr function does not work correctly, since the sin_len/sin6_len fields are not set. The attached patch solves the problem for me, and should not hinder on other operating systems.

File attachments

@vicuna
Copy link
Author

vicuna commented May 31, 2009

Comment author: @xavierleroy

Unfortunately, the patch does hinder very much on other operating systems: for instance, Linux has neither sin_len nor sin6_len fields.

Concerning sin_len, Stevens says that it's not part of POSIX 1g and that it's for kernel internal use -- the kernel should not expect the user to set it. If NetBSD thinks otherwise, it's a NetBSD problem.

Concerning sin_len6, there's an #ifdef to be put around assignments to sin6_len.

@vicuna
Copy link
Author

vicuna commented May 31, 2009

Comment author: jaap

What happens is that without the patch, something like

let sa = ADDR_INET (inet_addr_of_string some_ip_address, 80) in
getnameinfo sa [];;

always gives a Not_found exception, whereas with the patch, it correctly returns.

The NetBSD implementation of getnameinfo explicitly checks for sockaddr.sa_len not being 0, otherwise it fails. This seems to be a BSDism, though, as you say. Would it help if I put an #ifdef SYS_bsd around it and changed the assignments to sin_len/sin6_len to sa_len?

@vicuna
Copy link
Author

vicuna commented May 31, 2009

Comment author: @xavierleroy

I agree with you that setting the sa_len field in get_sockaddr() should be enough.

A Web search for "getnameinfo sa_len" reveals the portability nightmare: some systems (BSD and perhaps others) have sa_len and check it in getnameinfo(), and other systems (Linux and perhaps others) do not have sa_len at all. So it's #ifdef time...

I don't think #ifdef SYS_bsd is appropriate: the SYS_xxx Caml macros are not defined when compiling otherlibs/, and other systems beyond BSD may need sa_len to be set. A special "configure" test is probably needed.

@vicuna
Copy link
Author

vicuna commented Oct 17, 2009

Comment author: areilles

Looking at the issue, it appears that a correct fix is to use #ifdef SIN6_LEN to know whether the OS does use BSD 4.3 sockets, or BSD 4.4 sockets.
See RFC 2553, at the top of page 8:
" The only differences between this data structure and the 4.3BSD
variant are the inclusion of the length field, and the change of the
family field to a 8-bit data type. The definitions of all the other
fields are identical to the structure defined in the previous
section.

Systems that provide this version of the sockaddr_in6 data structure
must also declare SIN6_LEN as a result of including the
<netinet/in.h> header. This macro allows applications to determine
whether they are being built on a system that supports the 4.3BSD or
4.4BSD variants of the data structure.
"
http://www.ietf.org/rfc/rfc2553.txt

The patch-ba file seems to fix the issue both for netbsd and solaris (which does not have sa_len)

@vicuna
Copy link
Author

vicuna commented Oct 18, 2009

Comment author: @xavierleroy

Looking at the issue, it appears that a correct fix is to use #ifdef SIN6_LEN
to know whether the OS does use BSD 4.3 sockets, or BSD 4.4 sockets.
See RFC 2553, at the top of page 8:

Thanks for the suggestion. A Google search suggests to use #ifdef SIN_LEN to check for sin_len, and #ifdef SIN6_LEN to check for sin6_len. (It makes sense.)
Will do this in time for release 3.11.2.

The patch-ba file seems to fix the issue both for netbsd and solaris
(which does not have sa_len)

patch-ba changes Makefile but not socketaddr.c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants