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

[Jaroslav Bachorik]

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.

-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-
>