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

[Roger Riggs]

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.
>