RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

[Jaroslav Bachorik]

On 6.2.2015 06:16, David Holmes wrote:
> On 6/02/2015 1:17 AM, Jaroslav Bachorik wrote:
>> On 5.2.2015 14:55, Staffan Larsen wrote:
>>> Jaroslav,
>>>
>>> This looks good, just a few small comments:
>>>
>>> writableFlags.hpp:
>>>
>>> L25 #ifndef WritableFlags_HPP
>>> L26 #define WritableFlags_HPP
>>> L96 #endif /* WritableFlags_HPP */
>>> Should be SHARE_VM_SERVICES_WRITABLEFLAG_HPP
>>>
>>> L32: I don’t like inverted logic such as “NO_ERR”. I would prefer
>>> “SUCCESS” instead.
>>>
>>> writableFlags.cpp:
>>>
>>> L25: #include statements should be in alphabetical order if possible
>>> L166: (nit) missing empty line
>>> L155: I notice the Flag class uses “writeable” (with an ‘e’) and this
>>> class uses “writable” (without ‘e’) - My english isn’t good enough to
>>> tell if there is any difference.
>>
>> They should be equal in meaning. According to GooglBattle [1] the form
>> 'writable' is far wider spread but I would leave the decision to a
>> native speaker.
>>
>> [1]
>> http://www.googlebattle.com/index.php?domain=writeable&domain2=writable&submit=Go!
>>
>>
>>
>>> L194: Unused variable “succeed"
>>
>> The rest of the comments will be addressed. I will wait for eg. David to
>> comment on 'writable' vs. 'writeable' before regenerating the webrev.
>
> Take your pick. Both are used in the hotspot sources eg globals.hpp
> contains is_writeable() and check_writable() :(
>
> It is on my TODO list to review this ASAP.

Taffan's comments addressed - 
http://cr.openjdk.java.net/~jbachorik/8067447/webrev.03

-JB-

>
> David
>
>
>
>> -JB-
>>
>>>
>>>
>>> Thanks,
>>> /Staffan
>>>
>>>> On 4 feb 2015, at 17:55, Jaroslav Bachorik
>>>> <jaroslav.bachorik at oracle.com> wrote:
>>>>
>>>> Hi, this is a round 2 review of the proposed solution.
>>>>
>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8067447
>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8067447/webrev.02
>>>>
>>>> This patch is a precursor for implementing
>>>> https://bugs.openjdk.java.net/browse/JDK-8054890 which itself is a
>>>> part of JEP-228 (https://bugs.openjdk.java.net/browse/JDK-8043764).
>>>>
>>>> Here, the code related to manipulating JVM flags is extracted to a
>>>> separate WritableFlags class and the codebease is adjusted to use
>>>> this class.
>>>>
>>>> I didn't touch the original (nonstandard) handling of the DTrace
>>>> specific flags in the Solaris specific attachListener.cpp
>>>> implementation to keep the change simple and focused.
>>>>
>>>> Thanks,
>>>>
>>>> -JB-
>>>
>>