RFR JDK-8005120

[John Zavgren]

Greetings:
I would like you to review recent changes that I made to our native 
socket code for the purpose of eliminating compiler warnings.

These changes boil down to passing parameters with the correct type to 
various system calls as well as one "opportunistic" change that corrects 
the implementation of the macro: FT2INT64, that's used by windows:
  #define FT2INT64(ft) \
-        ((long)(ft).dwHighDateTime << 32 | (long)(ft).dwLowDateTime)
+        ((INT64)(ft).dwHighDateTime << 32 | (INT64)(ft).dwLowDateTime)

The webrev image is located at:
http://cr.openjdk.java.net/~jzavgren/8005120/webrev.07/ 
<http://cr.openjdk.java.net/%7Ejzavgren/8005120/webrev.07/>

Thank you!
John Zavgren
john.zavgren at oracle.com

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.hegarty at oracle.com
>> To: david.holmes at oracle.com
>> Cc: Alan.Bateman at oracle.com, serviceability-dev at openjdk.java.net, john.zavgren at oracle.com, net-dev at 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/


-- 
John Zavgren
john.zavgren at oracle.com
603-821-0904
US-Burlington-MA

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130117/e7902dfa/attachment-0001.html