Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004810OCamlOCaml generalpublic2009-05-29 11:452009-10-18 10:37
Reporterjaap 
Assigned Toxleroy 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.11.1+rc0 
Target VersionFixed in Version3.11.2+dev 
Summary0004810: small patch for unix/socketaddr.c
DescriptionUnder 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.
TagsNo tags attached.
Attached Files? file icon patch-ca [^] (788 bytes) 2009-05-29 11:45 [Show Content]
? file icon patch-ba [^] (1,449 bytes) 2009-10-17 14:24 [Show Content]

- Relationships

-  Notes
(0004975)
xleroy (administrator)
2009-05-31 12:20

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.
(0004976)
jaap (reporter)
2009-05-31 15:13

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?
(0004977)
xleroy (administrator)
2009-05-31 16:09

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.
(0005135)
areilles (reporter)
2009-10-17 14:30

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)
(0005136)
xleroy (administrator)
2009-10-18 10:37

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

- Issue History
Date Modified Username Field Change
2009-05-29 11:45 jaap New Issue
2009-05-29 11:45 jaap File Added: patch-ca
2009-05-31 12:20 xleroy Note Added: 0004975
2009-05-31 12:20 xleroy Status new => feedback
2009-05-31 15:13 jaap Note Added: 0004976
2009-05-31 16:09 xleroy Note Added: 0004977
2009-05-31 16:09 xleroy Assigned To => xleroy
2009-05-31 16:09 xleroy Status feedback => acknowledged
2009-10-17 14:24 areilles File Added: patch-ba
2009-10-17 14:30 areilles Note Added: 0005135
2009-10-18 10:37 xleroy Note Added: 0005136
2009-10-18 10:37 xleroy Status acknowledged => closed
2009-10-18 10:37 xleroy Resolution open => fixed
2009-10-18 10:37 xleroy Fixed in Version => 3.11.2+dev


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker