Re: RFR 8065783: DCMD parser fails to recognize one character argument when it's positioned last

2014-11-28 Thread frederic parain

Thank you for fixing this.

Looks good to me.

+1 on Erik's comments.

Fred

On 27/11/2014 15:25, Erik Gahlin wrote:

Looks ok,

Things that could be fixed,  but not necessary.

src/share/vm/services/diagnosticFramework.cpp : 63 // skipping spaces
could be changed to skipping delimiters

src/share/vm/services/diagnosticFramework.cpp : 66 _cursor = _len - 1;
Could be removed, since it must be true to enter if clause.

Erik

Jaroslav Bachorik skrev 2014-11-26 14:23:

Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8065783
Webrev: http://cr.openjdk.java.net/~jbachorik/8065783/webrev.00

Currently, the DCMD parser fails to recognize the trailing one
character long arguments. This is caused by the parser ignoring the
last character when immediately following a delimiter char.

The fix itself consists of checking whether the last character is
indeed a delimiter char
(src/share/vm/services/diagnosticFramework.cpp#68).

The rest of the patch deals with testing - the WhiteBox support needs
to be enhanced to handle DCMD arguments (currently only options) and
custom delimiters (currently only comma).

Thanks,

-JB-




--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com


Re: RFR 8065783: DCMD parser fails to recognize one character argument when it's positioned last

2014-11-28 Thread Jaroslav Bachorik

Thanks Frederic, Erik!

The update webrev is here: 
http://cr.openjdk.java.net/~jbachorik/8065783/webrev.01


May I have (R)eviewer taking a stab at this?

Thanks,

-JB-

On 11/28/2014 02:04 PM, frederic parain wrote:

Thank you for fixing this.

Looks good to me.

+1 on Erik's comments.

Fred

On 27/11/2014 15:25, Erik Gahlin wrote:

Looks ok,

Things that could be fixed,  but not necessary.

src/share/vm/services/diagnosticFramework.cpp : 63 // skipping spaces
could be changed to skipping delimiters

src/share/vm/services/diagnosticFramework.cpp : 66 _cursor = _len - 1;
Could be removed, since it must be true to enter if clause.

Erik

Jaroslav Bachorik skrev 2014-11-26 14:23:

Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8065783
Webrev: http://cr.openjdk.java.net/~jbachorik/8065783/webrev.00

Currently, the DCMD parser fails to recognize the trailing one
character long arguments. This is caused by the parser ignoring the
last character when immediately following a delimiter char.

The fix itself consists of checking whether the last character is
indeed a delimiter char
(src/share/vm/services/diagnosticFramework.cpp#68).

The rest of the patch deals with testing - the WhiteBox support needs
to be enhanced to handle DCMD arguments (currently only options) and
custom delimiters (currently only comma).

Thanks,

-JB-








Re: RFR 8065783: DCMD parser fails to recognize one character argument when it's positioned last

2014-11-28 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan


 On 28 nov 2014, at 16:11, Jaroslav Bachorik jaroslav.bacho...@oracle.com 
 wrote:
 
 Thanks Frederic, Erik!
 
 The update webrev is here: 
 http://cr.openjdk.java.net/~jbachorik/8065783/webrev.01
 
 May I have (R)eviewer taking a stab at this?
 
 Thanks,
 
 -JB-
 
 On 11/28/2014 02:04 PM, frederic parain wrote:
 Thank you for fixing this.
 
 Looks good to me.
 
 +1 on Erik's comments.
 
 Fred
 
 On 27/11/2014 15:25, Erik Gahlin wrote:
 Looks ok,
 
 Things that could be fixed,  but not necessary.
 
 src/share/vm/services/diagnosticFramework.cpp : 63 // skipping spaces
 could be changed to skipping delimiters
 
 src/share/vm/services/diagnosticFramework.cpp : 66 _cursor = _len - 1;
 Could be removed, since it must be true to enter if clause.
 
 Erik
 
 Jaroslav Bachorik skrev 2014-11-26 14:23:
 Please, review the following change
 
 Issue : https://bugs.openjdk.java.net/browse/JDK-8065783
 Webrev: http://cr.openjdk.java.net/~jbachorik/8065783/webrev.00
 
 Currently, the DCMD parser fails to recognize the trailing one
 character long arguments. This is caused by the parser ignoring the
 last character when immediately following a delimiter char.
 
 The fix itself consists of checking whether the last character is
 indeed a delimiter char
 (src/share/vm/services/diagnosticFramework.cpp#68).
 
 The rest of the patch deals with testing - the WhiteBox support needs
 to be enhanced to handle DCMD arguments (currently only options) and
 custom delimiters (currently only comma).
 
 Thanks,
 
 -JB-
 
 
 



Re: RFR 8065783: DCMD parser fails to recognize one character argument when it's positioned last

2014-11-27 Thread Erik Gahlin

Looks ok,

Things that could be fixed,  but not necessary.

src/share/vm/services/diagnosticFramework.cpp : 63 // skipping spaces
could be changed to skipping delimiters

src/share/vm/services/diagnosticFramework.cpp : 66 _cursor = _len - 1;
Could be removed, since it must be true to enter if clause.

Erik

Jaroslav Bachorik skrev 2014-11-26 14:23:

Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8065783
Webrev: http://cr.openjdk.java.net/~jbachorik/8065783/webrev.00

Currently, the DCMD parser fails to recognize the trailing one 
character long arguments. This is caused by the parser ignoring the 
last character when immediately following a delimiter char.


The fix itself consists of checking whether the last character is 
indeed a delimiter char 
(src/share/vm/services/diagnosticFramework.cpp#68).


The rest of the patch deals with testing - the WhiteBox support needs 
to be enhanced to handle DCMD arguments (currently only options) and 
custom delimiters (currently only comma).


Thanks,

-JB-