RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
Thanks for the review Roger !
From: Roger Riggs
Sent: Friday, January 20, 2017 1:49 AM
To: Amit Sapre; serviceability-dev@openjdk.java.net
Cc: Harsha Wardhana B; Dmitry Samersoff
Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0"
instead of assigned port
Hi Amit,
Looks fine to me.
Thanks, Roger
On 1/19/2017 6:54 AM, Amit Sapre wrote:
Hi,
I updated the parsing logic for extracting port number in test case. Here is
the updated webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8167337/webrev.01/
Thanks,
Amit
-Original Message-
From: Amit Sapre
Sent: Wednesday, January 18, 2017 2:09 PM
To: Roger Riggs; HYPERLINK
"mailto:serviceability-dev@openjdk.java.net"serviceability-dev@openjdk.java.net;
Harsha Wardhana B
Subject: RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
"0" instead of assigned port
Hello,
Looks like basic check on Jdp packet includes a check for
JMX_SERVICE_URL
https://java.se.oracle.com/source/xref/jdk9-
dev/jdk/test/sun/management/jdp/JdpTestCase.java#184
I feel we don't need any further check on jmx service url.
-Original Message-
From: Roger Riggs
Sent: Tuesday, January 17, 2017 10:05 PM
To: HYPERLINK
"mailto:serviceability-dev@openjdk.java.net"serviceability-dev@openjdk.java.net
Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP
broadcasts
"0" instead of assigned port
Hi,
yes, but the pattern looks for the ":" before and the "/" after the
zero.
It would not match the port ":00/" ; in this test code the URL is
assumed/known to be relatively well formed.
Roger
I kept the focus on what needs to be tested. This however doesn't mean
we shouldn't validate the url format.
I would prefer to do that in a separate test case all together.
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.
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
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.
Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
Hi Amit,
Looks fine to me.
Thanks, Roger
On 1/19/2017 6:54 AM, Amit Sapre wrote:
Hi,
I updated the parsing logic for extracting port number in test case. Here is
the updated webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8167337/webrev.01/
Thanks,
Amit
-Original Message-
From: Amit Sapre
Sent: Wednesday, January 18, 2017 2:09 PM
To: Roger Riggs; serviceability-dev@openjdk.java.net; Harsha Wardhana B
Subject: RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
"0" instead of assigned port
Hello,
Looks like basic check on Jdp packet includes a check for
JMX_SERVICE_URL
https://java.se.oracle.com/source/xref/jdk9-
dev/jdk/test/sun/management/jdp/JdpTestCase.java#184
I feel we don't need any further check on jmx service url.
-Original Message-
From: Roger Riggs
Sent: Tuesday, January 17, 2017 10:05 PM
To: serviceability-dev@openjdk.java.net
Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP
broadcasts
"0" instead of assigned port
Hi,
yes, but the pattern looks for the ":" before and the "/" after the
zero.
It would not match the port ":00/" ; in this test code the URL is
assumed/known to be relatively well formed.
Roger
I kept the focus on what needs to be tested. This however doesn't mean
we shouldn't validate the url format.
I would prefer to do that in a separate test case all together.
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.
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
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.
RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
Hi,
I updated the parsing logic for extracting port number in test case. Here is
the updated webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8167337/webrev.01/
Thanks,
Amit
> -Original Message-
> From: Amit Sapre
> Sent: Wednesday, January 18, 2017 2:09 PM
> To: Roger Riggs; serviceability-dev@openjdk.java.net; Harsha Wardhana B
> Subject: RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
> "0" instead of assigned port
>
> Hello,
>
> Looks like basic check on Jdp packet includes a check for
> JMX_SERVICE_URL
> https://java.se.oracle.com/source/xref/jdk9-
> dev/jdk/test/sun/management/jdp/JdpTestCase.java#184
>
> I feel we don't need any further check on jmx service url.
>
>
> > -Original Message-
> > From: Roger Riggs
> > Sent: Tuesday, January 17, 2017 10:05 PM
> > To: serviceability-dev@openjdk.java.net
> > Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP
> broadcasts
> > "0" instead of assigned port
> >
> > Hi,
> >
> > yes, but the pattern looks for the ":" before and the "/" after the
> > zero.
> > It would not match the port ":00/" ; in this test code the URL is
> > assumed/known to be relatively well formed.
> >
> > Roger
>
> I kept the focus on what needs to be tested. This however doesn't mean
> we shouldn't validate the url format.
> I would prefer to do that in a separate test case all together.
>
> >
> >
> > 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.
> > >>>
> > >>>
> > >>>
> > >>> 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
> > >>
> > >>
> > >>>
> > >>>
> > >>>
> > >>> 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.
> > >>>
> > >>
> > >
> >
RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
Hello,
Looks like basic check on Jdp packet includes a check for JMX_SERVICE_URL
https://java.se.oracle.com/source/xref/jdk9-dev/jdk/test/sun/management/jdp/JdpTestCase.java#184
I feel we don't need any further check on jmx service url.
> -Original Message-
> From: Roger Riggs
> Sent: Tuesday, January 17, 2017 10:05 PM
> To: serviceability-dev@openjdk.java.net
> Subject: Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts
> "0" instead of assigned port
>
> Hi,
>
> yes, but the pattern looks for the ":" before and the "/" after the
> zero.
> It would not match the port ":00/" ; in this test code the URL is
> assumed/known to be relatively well formed.
>
> Roger
I kept the focus on what needs to be tested. This however doesn't mean we
shouldn't validate the url format.
I would prefer to do that in a separate test case all together.
>
>
> 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.
> >>>
> >>>
> >>>
> >>> 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
> >>
> >>
> >>>
> >>>
> >>>
> >>> 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.
> >>>
> >>
> >
>
Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
Hi,
yes, but the pattern looks for the ":" before and the "/" after the zero.
It would not match the port ":00/" ; 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.
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
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.
Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
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.
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
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.
Re: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
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.
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
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.
RE: RFR : JDK-8167337 - When jmxremote.port=0, JDP broadcasts "0" instead of assigned port
> 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.
>
>
>
> 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");
> }
>
>
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.
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.
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.
