harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.com>
Subject Re: [general] Code Freeze for Milestone 5.0M14 and 6.0M2
Date Fri, 21 May 2010 01:12:24 GMT
On 2010-05-20 18:19, Mark Hindess wrote:
>
> In message<4BF504A6.3020902@gmail.com>, Regis writes:
>>
>> On 2010-05-20 17:19, Mark Hindess wrote:
>>>
>>> In message<4BF4F9A4.6050201@gmail.com>, Regis writes:
>>>>
>>>> On 2010-05-20 16:46, Mark Hindess wrote:
>>>>>
>>>>> In message<4BF4EC09.3090407@gmail.com>, Regis writes:
>>>>>>
>>>>>> On 2010-05-19 20:45, Catherine Hope wrote:
>>>>>>> I also see these 2 tests failing in the same way.  It seems to
be
>>>>>>> caused by Regis' commit 944119 "SocketChannelImpl.SocketAdapter's
>>>>>>> remote address should be updated after channel connected" on
>>>>>>> 14/05/2010.  It could be that the tests are invalid though -
>>>>>>> the first one is checking that the SocketAddress returned by
>>>>>>> getRemoteAddress() is a different object to the one that was
passed
>>>>>>> to connect(), though as SocketAddress is immutable would this
cause
>>>>>>> a problem?
>>>>>>>
>>>>>>
>>>>>> The test case expect behaviors to be exactly same with RI, but I
don't
>>>>>> think it's a big deal, I intend to delete this assert.
>>>>>>
>>>>>> for - testSocket_NonBlock_BasicStatusAfterConnect, isConnected()
>>>>>> should check first, following patch can fix it. And again, the test
>>>>>> failed at same line as BasicStatusAfterConnect.
>>>>>>
>>>>>> Following patch can fix these failures, I'd like to commit it if
>>>>>> second committer agree this.
>>>>>
>>>>> -1
>>>>>
>>>>> I am +1 for the patch to SocketChannelImpl.java because this fixes
>>>>> behaviour.
>>>>>
>>>>> However, I am -1 for the patch to SocketChannelTest.java because we
>>>>> don't fix broken tests during code freeze.
>>>>
>>>> All right, how about this (create new InetSocketAddress instance everytime
>> ):
>>>
>>> -1
>>>
>>> Fix the behaviour for testSocket_NonBlock_BasicStatusAfterConnect.
>>>
>>> Do not make any changes for the other failure - i.e. leave it failing -
>>> and remove it after the code freeze.
>>
>> Do you mean this?
>
> Yes.  (I think "I am +1 for the patch to SocketChannelImpl.java" above
> relates to the same changes.)
>
> Regards,
>   Mark.
>
>> Index:
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SocketChanne
>> lImpl.java
>> =====================================================================
>> ---
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SocketChanne
>> lImpl.java
>> +++
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SocketChanne
>> lImpl.java
>> @@ -1008,6 +1008,10 @@ class SocketChannelImpl extends SocketChannel implemen
>> ts
>> FileDescriptorHandler {
>>
>>            @Override
>>            public InetAddress getInetAddress() {
>> +            if (!isConnected()) {
>> +                return null;
>> +            }
>> +
>>                if (channel.connectAddress == null&&  super.getInetAddress()
!=
>>
>> null) {
>>                    channel.connectAddress = new
>> InetSocketAddress(super.getInetAddress(), super.getPort());
>>                }
>> @@ -1019,6 +1023,9 @@ class SocketChannelImpl extends SocketChannel implement
>> s
>> FileDescriptorHandler {
>>
>>            @Override
>>            public SocketAddress getRemoteSocketAddress() {
>> +            if (!isConnected()) {
>> +                return null;
>> +            }
>>                if (channel.connectAddress == null&&  super.getInetAddress()
!=
>>
>> null) {
>>                    channel.connectAddress = new
>> InetSocketAddress(super.getInetAddress(), super.getPort());
>>                }
>> @@ -1027,6 +1034,9 @@ class SocketChannelImpl extends SocketChannel implement
>> s
>> FileDescriptorHandler {
>>
>>            @Override
>>            public int getPort() {
>> +            if (!isConnected()) {
>> +                return 0;
>> +            }
>>                if (channel.connectAddress == null&&  super.getInetAddress()
!=
>>
>> null) {
>>                    channel.connectAddress = new
>> InetSocketAddress(super.getInetAddress(), super.getPort());
>>                }
>>
>>
>> --
>> Best Regards,
>> Regis.
>>
>
>
>

Thanks Mark. The fix was applied at trunk r946842.

-- 
Best Regards,
Regis.

Mime
View raw message