> Hi Amit,
>
> In JdpJmxRemoteDynamicPortTestCase:48 needs null/empty check for jmx
> url.
Ok. Will add this.
>
> 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(':');
> }
>
> if(jmxurl.indexOf('/') == -1) {
> throw new RuntimeException("Test failed : Invalid
> JMXServiceURL");
> }
>
> String portStr = jmxurl.substring(0,jmxurl.indexOf('/'));
> int port = Integer.parseInt(portStr);
> if( port == 0 ) {
> throw new RuntimeException("Test failed : Zero port for
> jmxremote");
> }
>
> </code>
I had thought if this and then found out that checking with ":" character first
will
bring out all these uncertainties. That's the reason , I first split with "/",
whose count is constant for either form of jmx url.
So accessing token[6] will always give the substring containing remote port.
Amit
> -----Original Message-----
> From: Amit Sapre
> Sent: Monday, January 16, 2017 11:17 AM
> To: Dmitry Samersoff; serviceability-dev
> Subject: RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
> "0" instead of assigned port
>
> 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.
