RFR: 8080947: Add uint as a valid VM flag type

[Dmitry Dmitriev]

Thanks! No, not need another webrev. Also, I am not a reviewer.

Regards,
Dmitry

On 04.06.2015 15:09, David Lindholm wrote:
> 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, size_t len, 
>> uint* value, Flag::Flags origin) {
>>  739   Flag* result = Flag::find_flag(name, len);
>>  740   if (result == NULL) return false;
>>  741   if (!result->is_uint()) return false;
>>  742   uint old_value = result->get_uint();
>>  743   trace_flag_changed<EventUnsignedIntFlagChanged, u4>(name, 
>> old_value, *value, origin);
>>  744   result->set_int(*value);
>>  745   *value = old_value;
>>  746   result->set_origin(origin);
>>  747   return true;
>>  748 }
>>  749
>>  750 void CommandLineFlagsEx::uintAtPut(CommandLineFlagWithType flag, 
>> uint value, Flag::Flags origin) {
>>  751   Flag* faddr = address_of_flag(flag);
>>  752   guarantee(faddr != NULL && faddr->is_uint(), "wrong flag type");
>>  753   trace_flag_changed<EventUnsignedIntFlagChanged, 
>> u4>(faddr->_name, faddr->get_uint(), value, origin);
>>  754   faddr->set_int(value);
>>  755   faddr->set_origin(origin);
>>  756 }
>>
>> On lines 744 and 754 "set_int" method is called for unsigned flags. 
>> Probably "set_uint" 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->is_int()) {
>>>>>> +     global->value.j = (jlong)flag->get_int();
>>>>>> +     global->type = JMM_VMGLOBAL_TYPE_JLONG;
>>>>>> +   } else if (flag->is_uint()) {
>>>>>> +     global->value.j = (jlong)flag->get_uint();
>>>>>> +     global->type = JMM_VMGLOBAL_TYPE_JLONG;
>>>>>>
>>>>>> These should be JINT types not JLONG.
>>>>>>
>>>>>> Cheers,
>>>>>> David H.
>>>>>> -------
>>>>>>
>>>>>>>
>>>>>>> Testing: Passed JPRT
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>
>>>>
>>>
>>
>