RFR: 8264285: Do not support FLAG_SET_XXX for VM flags of string type

[Ioi Lam]

On Fri, 26 Mar 2021 16:21:03 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> We have two versions of `JVMFlagAccess::ccstrAtPut()` that are slightly different.
> 
> The following version is supposed to be used only by the `FLAG_SET_{CMDLINE,ERGO,MGMT}` macros. However, it's not used anywhere in the HotSpot source code:
> 
> JVMFlag::Error JVMFlagAccess::ccstrAtPut(JVMFlagsEnum flag, ccstr value, JVMFlagOrigin origin) {
>   JVMFlag* faddr = JVMFlag::flag_from_enum(flag);
>   assert(faddr->is_ccstr(), "wrong flag type");
>   ccstr old_value = faddr->get_ccstr();
>   trace_flag_changed<ccstr, EventStringFlagChanged>(faddr, old_value, value, origin);
>   char* new_value = os::strdup_check_oom(value);
>   faddr->set_ccstr(new_value);
>   if (!faddr->is_default() && old_value != NULL) {
>     // Prior value is heap allocated so free it.
>     FREE_C_HEAP_ARRAY(char, old_value);
>   }
>   faddr->set_origin(origin);
>   return JVMFlag::SUCCESS;
> }
> 
> It's not clear whether this unused version is actually correct since the last JVMFlag rewrite in [JDK-8081833](https://bugs.openjdk.java.net/browse/JDK-8081833), due to complete lack of testing. Let's remove this version to simplify code maintenance.
> 
> If you need to modify flags of the string type, do not use `FLAG_SET_{CMDLINE,ERGO,MGMT}`. (A `static_assert` is added to prevent this). Instead, use the remaining version of `JVMFlagAccess::ccstrAtPut()`.

> > If you need to modify flags of the string type, do not use `FLAG_SET_{CMDLINE,ERGO,MGMT}`. (A `static_assert` is added to prevent this). Instead, use the remaining version of `JVMFlagAccess::ccstrAtPut()`.
> 
> ... and using ccstrAtPut doesn't update the origin of the flag as might
> be desired when using the macros.

This is the version I removed:

  static JVMFlag::Error ccstrAtPut(JVMFlagsEnum flag, ccstr value, JVMFlagOrigin origin);

This is the remaining version:

  static JVMFlag::Error ccstrAtPut(JVMFlag* flag, ccstr* value, JVMFlagOrigin origin);

So they are practically the same API. The origin is changed in both. The only difference is they have unobvious subtle difference in how they handle the buffer allocation.

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

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