RFR: 8264285: Clean the modification of ccstr JVM flags

[Coleen Phillimore]

On Mon, 29 Mar 2021 21:35:52 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> There are two versions of JVMFlagAccess::ccstrAtPut() for modifying JVM flags of the ccstr type (i.e., strings).
> 
> - One version requires the caller to free the old value, but some callers don't do that (writeableFlags.cpp).
> - The other version frees the old value on behalf of the caller. However, this version is accessible only via FLAG_SET_XXX macros and is currently unused. So it's unclear whether it actually works.
> 
> We should combine these two versions into a single function, fix problems in the callers, and add test cases. The old value should be freed automatically, because typically the caller isn't interested in the old value.
> 
> Note that the FLAG_SET_XXX macros do not return the old value. Requiring the caller of FLAG_SET_XXX to free the old value would be tedious and error prone.

I had a question but overall nice cleanup!

src/hotspot/share/runtime/flags/debug_globals.hpp line 38:

> 36: // have any MANAGEABLE flags of the ccstr type, but we really need to
> 37: // make sure the implementation is correct (in terms of memory allocation)
> 38: // just in case someone may add such a flag in the future.

Could you have just added a develop flag to the manageable flags instead?

src/hotspot/share/runtime/flags/jvmFlagAccess.cpp line 327:

> 325:   // The callers typically don't care what the old value is.
> 326:   // If the caller really wants to know the old value, read it (and make a copy if necessary)
> 327:   // before calling this API.

good comment!

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3254