John, socket_md.c:88 (and other similar places)
socklen_t nBytes should be size_t nbytes Otherwise looks good for me. -Dmitry On 2012-12-26 23:56, John Zavgren wrote: > Greetings: > I modified the windows code so that it uses socklen_t to specify the > lengths of data structures, parameter lengths, etc. instead of ints > (which can be negative.) > > http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/webrev.02/ > > Thanks! > John Zavgren > > On 12/20/2012 05:47 PM, John Zavgren wrote: >> Greetings: >> >> I modified my changes so that windows knows the definition of the >> POSIX data type: socklen_t, and now all the system calls are using the >> "doctrinaire" data types. >> >> Please consider the following update. >> >> http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/webrev.01/ >> >> Thanks! >> John Zavgren >> >> >> >> On 20/12/2012 13:49, John Zavgren wrote: >>> Greetings: >>> >>> I agree that the "correct" way to fix this problem is to use POSIX >>> data types, e.g., socklen_t. However, when I switch to the >>> doctrinaire data type, the build fails on windows machines: >>> ------------- build monologue ----- >>> c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) >>> : error C2146: syntax error : missing ')' before identifier 'len' >>> c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) >>> : error C2081: 'socklen_t' : name in formal parameter list illegal >>> c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) >>> : error C2061: syntax error : identifier 'len' >>> c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) >>> : error C2059: syntax error : ';' >>> c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) >>> : error C2059: syntax error : ')' >>> .... >>> ------------- build monologue ----- >>> >>> I used alternative types, e.g., uint32_t, etc. as a way to avoid the >>> limitations of windows. >>> What is the recommended way to accommodate this windows limitation? >>> Shall I use a typedef statement to define "socklen_t"? >> We don't suffer from this issue in the networking native code. The unix >> and windows implementations are distinct. >> >> I see the vm defines socklen_t in a windows specific header, >> hotspot/src/os/windows/vm/jvm_windows.h, as >> typedef int socklen_t; >> >> ...and it is then used in shared code, like jvm.cpp, and the hpi, by >> optionally including >> >> #ifdef TARGET_OS_FAMILY_windows >> # include "jvm_windows.h" >> #endif >> >> We could use a similar, but more simplistic, approach here. >> >> -Chris. >> >>> Thanks! >>> >>> >>> ----- Original Message ----- >>> From: chris.hega...@oracle.com >>> To: david.hol...@oracle.com >>> Cc: alan.bate...@oracle.com, serviceability-...@openjdk.java.net, >>> john.zavg...@oracle.com, net-dev@openjdk.java.net >>> Sent: Thursday, December 20, 2012 7:41:07 AM GMT -05:00 US/Canada >>> Eastern >>> Subject: Re: RFR JDK-8005120 >>> >>> On 19/12/2012 20:52, David Holmes wrote: >>>> Real sense of deja-vu here. Didn't we go through this same thing with >>>> the HPI socket routines? >>> Yes, and the networking native code too. >>> >>> I think it is best to use socklen_t for the unix code. From what I can >>> see making these changes, to use socklen_t, should be relatively >>> localized. >>> >>> -Chris. >>> >>>> Depending on the OS (and version?) we should be using socklen_t not int >>>> and not uint32_t. >>>> >>>> David >>>> >>>> On 20/12/2012 2:35 AM, Chris Hegarty wrote: >>>>> John, >>>>> >>>>> I grabbed your patch, and with it I now see different warnings. >>>>> >>>>> ../../../../src/share/transport/socket/socketTransport.c: In function >>>>> 'socketTransport_startListening': >>>>> ../../../../src/share/transport/socket/socketTransport.c:310:40: >>>>> warning: pointer targets in passing argument 3 of >>>>> 'dbgsysGetSocketName' >>>>> differ in signedness [-Wpointer-sign] >>>>> ../../../../src/share/transport/socket/sysSocket.h:58:5: note: >>>>> expected >>>>> 'uint32_t *' but argument is of type 'int *' >>>>> ../../../../src/share/transport/socket/socketTransport.c: In function >>>>> 'socketTransport_accept': >>>>> ../../../../src/share/transport/socket/socketTransport.c:371:33: >>>>> warning: pointer targets in passing argument 3 of 'dbgsysAccept' >>>>> differ >>>>> in signedness [-Wpointer-sign] >>>>> ../../../../src/share/transport/socket/sysSocket.h:41:5: note: >>>>> expected >>>>> 'uint32_t *' but argument is of type 'int *' >>>>> >>>>> Do you see these in your build? >>>>> >>>>> -Chris. >>>>> >>>>> On 12/19/2012 03:42 PM, Alan Bateman wrote: >>>>>> John - this is the debugger socket transport so cc'ing the >>>>>> serviceability-dev list as that is where this code is maintained. >>>>>> >>>>>> On 19/12/2012 15:36, John Zavgren wrote: >>>>>>> Greetings: >>>>>>> Please consider the following change to the two files: >>>>>>> src/share/transport/socket/sysSocket.h >>>>>>> src/solaris/transport/socket/socket_md.c >>>>>>> that eliminate compiler warnings that stem from the fact that the >>>>>>> variables that the native code passes to various system calls >>>>>>> were not >>>>>>> declared correctly. They were declared as integers, but they must be >>>>>>> "unsigned" integers because they are used to define buffer lengths. >>>>>>> Were one to supply a negative value as an argument, it would be cast >>>>>>> into an unsigned "Martian" value and there'd be (hopefully) a system >>>>>>> call error. >>>>>>> >>>>>>> Thanks! >>>>>>> John Zavgren >>>>>>> >>>>>>> http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/ > > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
