RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port

[Harsha Wardhana B]

Hi Roger,

Your approach is more elegant. However checking for ":0/" may not work 
as we can have non-zero port number that can end in 0.

Regards

Harsha


On Tuesday 17 January 2017 09:39 PM, Roger Riggs wrote:
> Hi Harsha,
>
> On 1/16/2017 1:21 AM, Harsha Wardhana B wrote:
>> Hi Amit,
>>
>> In JdpJmxRemoteDynamicPortTestCase:48 needs null/empty check for jmx 
>> url.
>>
>> JdpJmxRemoteDynamicPortTestCase:49, array length needs to checked 
>> before accessing index at token[6].
>>
>> It is possible that port number need not always be present at given 
>> index and hence we may have to follow different approach to extract 
>> port number. Please check if approach below works.
>>
>> <code>
>>
>>         int idx = jmxurl.indexOf(':');
>>         while (idx != -1) {
>>             jmxurl = jmxurl.substring(idx+1);
>>             idx = jmxurl.indexOf(':');
>>         }
> This loop would very eagerly find the last ":" in the string even it 
> was well past the host/port field.
> String.lastIndex would be equivalent.
>>
>>         if(jmxurl.indexOf('/') == -1) {
>>             throw new RuntimeException("Test failed : Invalid 
>> JMXServiceURL");
>>         }
> It would be more efficient to compare the index of the '/' after the 
> last ":" than to re-create new substrings.
> int colon = jmxurl.lastIndexOf(':');
> int slash = jmxurl.indexOf('/', colon);
> int port = Integer.parseInt(jmxurl, colon + 1, slash, 10);
>
>>
>>         String portStr = jmxurl.substring(0,jmxurl.indexOf('/'));
>>         int port = Integer.parseInt(portStr);
>>         if( port == 0 ) {
>>             throw new RuntimeException("Test failed : Zero port for 
>> jmxremote");
>>         }
> Or It might be just as effective to just to check if ":0/" is present.
> if (jmxurl.contains(":0/")) {...}
>
> $.02, Roger
>
>
>>
>> </code>
>>
>> Regards
>>
>> Harsha
>>
>>
>> On Monday 16 January 2017 11:16 AM, Amit Sapre wrote:
>>> Thanks Dmitry for the review.
>>>
>>> Can I have one more reviewer for this fix ?
>>>
>>> Thanks,
>>> Amit
>>>
>>>> -----Original Message-----
>>>> From: Dmitry Samersoff
>>>> Sent: Sunday, January 15, 2017 4:49 PM
>>>> To: Amit Sapre; serviceability-dev
>>>> Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
>>>> "0" instead of assigned port
>>>>
>>>> Amit,
>>>>
>>>> Changes looks good to me.
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2017-01-13 09:17, Amit Sapre wrote:
>>>>> Hello,
>>>>>
>>>>>
>>>>>
>>>>> Please review the fix for JDK-8167337
>>>>>
>>>>>
>>>>>
>>>>> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8167337
>>>>>
>>>>> Webrev :
>>>>> http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8167337/webrev.00/
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Amit
>>>>>
>>>>
>>>> -- 
>>>> Dmitry Samersoff
>>>> Oracle Java development team, Saint Petersburg, Russia
>>>> * I would love to change the world, but they won't give me the 
>>>> sources.
>>
>