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

[Roger Riggs]

Hi,

yes, but the pattern looks for the ":" before and the "/" after the zero.
It would not match the port ":000000/" ; in this test code the URL is 
assumed/known to be relatively well formed.

Roger


On 1/17/2017 11:26 AM, Harsha Wardhana B wrote:
> 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.
>>>
>>
>