RFR(XS): 8212883: Setting a double manageable flag with jcmd/jinfo crashes the JVM (original) (raw)

David Holmes david.holmes at oracle.com
Wed Oct 24 06:56:51 UTC 2018


Hi Thomas,

On 24/10/2018 4:32 PM, Thomas Stüfe wrote:

Hi,

I am really not sure about the implicit boolean comparison of the sscanf return value (not only for this but the other flag variants too).

It's checking for 0 or 1 matches. But yes hotspot style says it should be an explicit check against 0 or 1.

sscanf may return 0-n (for 0-n matched items) and EOF in case of an error. Is EOF always 0? Otherwise, to be sure, I would compare the return value with 1 since we expect 1 item to match.

That's probably technically safer, though for sscanf I don't see how you can return EOF unless the arg is an empty string - even then that may simply constitute 0 matches.

Cheers, David

Just my 5c.

Cheers, Thomas

On Wed, Oct 24, 2018 at 1:16 AM David Holmes <david.holmes at oracle.com> wrote: Hi Tony, On 24/10/2018 8:13 AM, Tony Printezis wrote: Webrev here:

http://cr.openjdk.java.net/~tonyp/8212883/webrev.0/ Currently, HotSpot doesn’t actually have any double manageable flags, which is why I think no-one has hit this before. I recently added a couple to our own builds and I noticed that setting them is not handled correctly in the VM. The fix is pretty trivial (mostly cut-and-paste from what the code does for the other types). I agree the fix is pretty obvious and straight-forward. I tested it by introducing a dummy double manageable flag and I can set it with jinfo/jcmd and jconsole (these cover all the various paths in the changes). Is it worth expanding the serviceability/attach/AttachSetGetFlag.java test to also get/set a double flag (I’d need to introduce a dummy double manageable flag to do that though)? I hate to see new code untested ... but then it seems we don't have tests for all the existing types of flags anyway. Reviewed. Thanks, David Regards, Tony

————— Tony Printezis | @TonyPrintezis | tprintezis at twitter.com



More information about the hotspot-runtime-dev mailing list