RFR: 8080947: Add uint as a valid VM flag type (original) (raw)

David Lindholm david.lindholm at oracle.com
Thu Jun 4 12:09:28 UTC 2015

Dmitry,

Thanks, you are correct. Fixed now. Do you need another webrev?

Thanks, David

On 2015-06-04 14:00, Dmitry Dmitriev wrote:

Hello David,

In src/share/vm/runtime/globals.cpp module: 738 bool CommandLineFlags::uintAtPut(const char* name, sizet len, uint* value, Flag::Flags origin) { 739 Flag* result = Flag::findflag(name, len); 740 if (result == NULL) return false; 741 if (!result->isuint()) return false; 742 uint oldvalue = result->getuint(); 743 traceflagchanged<EventUnsignedIntFlagChanged, u4>(name, oldvalue, *value, origin); 744 result->setint(*value); 745 *value = oldvalue; 746 result->setorigin(origin); 747 return true; 748 } 749 750 void CommandLineFlagsEx::uintAtPut(CommandLineFlagWithType flag, uint value, Flag::Flags origin) { 751 Flag* faddr = addressofflag(flag); 752 guarantee(faddr != NULL && faddr->isuint(), "wrong flag type"); 753 traceflagchanged<EventUnsignedIntFlagChanged,_ _u4>(faddr->name, faddr->getuint(), value, origin); 754 faddr->setint(value); 755 faddr->setorigin(origin); 756 } On lines 744 and 754 "setint" method is called for unsigned flags. Probably "setuint" method should be called? Regards, Dmitry On 04.06.2015 14:40, David Lindholm wrote: Hi,

Thanks. I have put up a new webrev with a few small changes: http://cr.openjdk.java.net/~david/JDK-8080947/webrev.hotspot.01/

Thanks, David On 2015-06-04 03:09, Coleen Phillimore wrote: David, If you have all the reviews you need, you can integrate. Gerard is working through review comments and has to retest, and is fine with merging with your change. Thanks everyone for all the thorough reviews of the command line validation change. Coleen On 6/3/15 4:51 AM, David Lindholm wrote: Hi David,

Thanks for looking at this. I have a few places where I convert uint and int to Java types, besides management.cpp also whitebox.cpp/java and VM.java . After discussing with several people we though it was most correct to go with JLONG as java type for both int and uint, since it is not certain that an uint will fit in a JINT and I wanted the same java type for both int and uint. I don't think the C spec specifies the size of int (please correct me if I'm wrong), so having JLONG as type for int and uint is safer than JINT. But I can change to JINT if you think that is better.

Thanks, David On 2015-06-03 10:01, David Holmes wrote: Hi David, On 28/05/2015 9:28 PM, David Lindholm wrote: Hi, Please review this patch that adds uint and int as valid VM flag types. This patch adds the possibility to specify VM flags with types int and uint, it does not change the type of any flags.

Webrev: http://cr.openjdk.java.net/~david/JDK-8080947/webrev.hotspot.00/ Webrev: http://cr.openjdk.java.net/~david/JDK-8080947/webrev.jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8080947 src/share/vm/services/management.cpp + } else if (flag->isint()) { + global->value.j = (jlong)flag->getint(); + global->type = JMMVMGLOBALTYPEJLONG; + } else if (flag->isuint()) { + global->value.j = (jlong)flag->getuint(); + global->type = JMMVMGLOBALTYPEJLONG; These should be JINT types not JLONG. Cheers, David H. ------- Testing: Passed JPRT Thanks, David

More information about the hotspot-dev mailing list