[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RFR JDK-8005120
From:       chris.hegarty () oracle ! com (Chris Hegarty)
Date:       2012-12-29 15:21:53
Message-ID: 50DF0A91.4090604 () oracle ! com
[Download RAW message or body]

John,

Windows x64 warnings, with your patch (below). size_t is a 64-bit value 
on Windows 64-bit, and recvfrom requires an int ( on Windows ).

../../../../src/windows/transport/socket/socket_md.c(181) : warning 
C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data
../../../../src/windows/transport/socket/socket_md.c(187) : warning 
C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data
../../../../src/windows/transport/socket/socket_md.c(192) : warning 
C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data
../../../../src/windows/transport/socket/socket_md.c(197) : warning 
C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data

I think in our usage it should be safe, and warning free, to revert 
size_t to an int, rather than have platform specific function definitions.

-Chris.

On 28/12/2012 15:49, John Zavgren wrote:
> Greetings:
>
> I modified windows source code files so they pass size_t for buffer
> sizes and socklen_t for address structures.
>
> http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/webrev.03/
>
> Please let me know what you think.
>
> Thanks!
>
> On 12/26/2012 02:56 PM, 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.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/
>>
>>
>
>

[prev in list] [next in list] [prev in thread] [next in thread]