RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

[Leonid Mesnik]

Hi

Currently  DCmdArgument<jlong> is used to parse any numeric values in dcmd framework including positive integer values for port, ttl etc.

Adding new type DCmdArgument<size_t> requires adding more specialized methods like
template <> void DCmdArgument<jlong>::parse_value(const char* str,
                                                  size_t len, TRAPS) {

and other to parse and validate any *integer* parameters. See
http://hg.openjdk.java.net/jdk/jdk/file/47ee6c00d27c/src/hotspot/share/services/diagnosticArgument.cpp#l112 <http://hg.openjdk.java.net/jdk/jdk/file/47ee6c00d27c/src/hotspot/share/services/diagnosticArgument.cpp#l112>

I think that it is easier to use single jlong for all integer like it is done now rather than adding more types.

One might said that it would be better to add size_t type and use it for all non-negative integers. It would be good improvement for all dcmd args parsing. As well as overall improvement of parameters validation. (dcmd often don't throw exception and just return in the case of incorrect arguments). But sees like separate effort for whole dcmd framework.
 

Leonid
> On Jun 7, 2019, at 1:37 AM, serguei.spitsyn at oracle.com wrote:
> 
> Hi Leonid,
> 
> It looks good to me.
> One minor comment on the src/hotspot/share/services/diagnosticCommand.?pp
> +  DCmdArgument<jlong> _granularity;
> 
> I'm curios if using size_t instead of jlong as in other files would be more unified.
> 
> Thanks,
> Serguei
> 
> 
> 
> On 6/6/19 17:36, Leonid Mesnik wrote:
>> Hi 
>> 
>> Could you please review following fix which verify parameter for Compiler.CodeHeap_Analytics command. So jcmd just exits instead of crashing target VM. Regression test was added, hs-tier1/2 passed. 
>> 
>> webrev: http://cr.openjdk.java.net/~lmesnik/8225388/webrev.00/ <http://cr.openjdk.java.net/~lmesnik/8225388/webrev.00/> 
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8225388 <https://bugs.openjdk.java.net/browse/JDK-8225388> 
>> 
>> Leonid 
>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190607/8a4bb596/attachment.html>