RFR(s): 8204958: Minor cleanups for the diagnostic framework

[Thomas Stüfe]

Hi Serguei,

On Thu, Jun 14, 2018 at 1:11 AM, serguei.spitsyn at oracle.com
<serguei.spitsyn at oracle.com> wrote:
> Hi Thomas,
>
> I looks good to me.
>
> A couple of minor comments.
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8204958-jcmd-cleanups/webrev.00/webrev/src/hotspot/share/services/diagnosticArgument.hpp.udiff.html
>
> -  void value_as_str(char *buf, size_t len) { return to_string(_value, buf,
> len);}
> +  void value_as_str(char *buf, size_t len) const { to_string(_value, buf,
> len);}
>
> I'm puzzled a little bit.
> How did the value_as_str() returned something before if the function has
> void return type?
>

Yes, I was surprised that this even compiled, but it did. I only did
catch it because CDT complained.

>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8204958-jcmd-cleanups/webrev.00/webrev/src/hotspot/share/services/diagnosticFramework.hpp.frames.html
>
>  170   DCmdArgumentInfo(const char* name, const char* description, const
> char* type,
>  171                    const char* default_string, bool mandatory, bool
> option,
>  172                    bool multiple, int position = -1)
>  173     : _name(name), _description(description), _type(type)
>  174     , _default_string(default_string), _mandatory(mandatory)
>  175     , _option(option), _multiple(multiple), _position(-1) {}
>
>
>   Would it be more safe for the _position initialization to be:
> _position(position)?
>

Yes, this is a real bug. Sorry, I'll fix it and comb the rest of the
change again.

Thanks, Thomas

> Thanks,
> Serguei
>
>
>
>
> On 6/13/18 06:41, Thomas Stüfe wrote:
>
> Hi all,
>
> while working on jcmd I saw some minor cleanup possibilities.
>
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8204958-jcmd-cleanups/webrev.00/webrev/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8204958
>
> Most of them are const correctness and using initializer lists instead
> of java-style member initialization in these C++ classes.
>
> Other changes:
>
> - Added a ResourceMark into the DCmd::parse_and_execute() loop
> - removed DCmdFactory::create_global_DCmd() (and following that,
> DCmdFactory::create_Cheap_instance()) since I did not find anyone
> using that function.
> - Also in DCmdFactory, I removed a number of setters which were never
> called and for attributes which should be immutable: hidden, flags,
> enabled.
>
> This is really a rather light cleanup. It did not touch any larger issues.
>
> Thank you,
>
> Thomas
>
>