Pls review 7091417: recvfrom's 6th input should be of type socklen_t (M)

[David Holmes]

Hi Paul,

A few comments below.

Cheers,
David

On 21/12/2011 12:19 AM, Paul Hohensee wrote:
> This is actually a bit broader scope than the Summary suggests, in that
> it takes into
> account the possibility that socklen_t might not be the same bit width as
> int and jint (which latter two are assumed synonymous in the jvm). I spent
> a bit of time with socket.h on linux/bsd/solaris to verify the actual
> argument
> types of the socket methods used by the jvm. I changed the os methods
> so that their formal argument types match socket.h and put that adapter
> code in jvm.cpp so as to have only a single copy of it.
>
> Webrev here
>
> http://cr.openjdk.java.net/~phh/7091417.00/

jvm.cpp:

I would have expected this to need explicit casts on some platforms, 
else I'd expect compiler warnings if a socklen_t is not an int:

3557 JVM_LEAF(jint, JVM_Accept(jint fd, struct sockaddr *him, jint *len))
3558   JVMWrapper2("JVM_Accept (0x%x)", fd);
3559   //%note jvm_r6
3560   socklen_t socklen = *len;
3561   jint result = os::accept(fd, him, &socklen);
3562   *len = socklen;
3563   return result;
3564 JVM_END

and similarly for JVM_Recvfrom / JVM_GetSockName / JVM_GetSockOpt

---

jvm_bsd.h:

I think the include for sys/socket.h needs to be moved to where the 
other includes start. It seems reasonable to me that its inclusion may 
be affected by the STDC macro stuff.

---

os_bsd.inline.hpp

I was wondering about the issue of the flags being int or uint. The 
linux manpage for recv:

http://linux.die.net/man/2/recv

sheds some light:

"The prototypes given above follow glibc2. The Single UNIX Specification 
agrees, except that it has return values of type ssize_t (while 4.x BSD 
and libc4 and libc5 all have int). The flags argument is int in 4.x BSD, 
but unsigned int in libc4 and libc5. The len argument is int in 4.x BSD, 
but size_t in libc4 and libc5. The addrlen argument is int * in 4.x BSD, 
libc4 and libc5. The present socklen_t * was invented by POSIX. See also 
accept(2). "

I'm assuming from this that libc4 and libc5 pre-date glibc2 and that we 
no longer have to worry about them?

---

os_<os>.inline.hpp

These files redundantly include sys/socket.h directly when it is now 
coming in via jvm_<os>.hpp via os.hpp

---

os_solaris.cpp

In recv/send etc do we have a potential issue with the casts from 
ssize_t to int on 64-bit systems?

---

jvm_windows.h

Shouldn't you include ws2tcpip.h to get socklen_t? That's what the JDK 
networking code uses. (Though as all this sock stuff is unused on win32 
I guess it doesn't really matter.)


> Thanks,
>
> Paul
>