RFR(XS): 8169734: Update uses of string "java.base" to macro

[Rachel Protacio]

Thanks, Dan! Ok, here's the webrev with all the instances changed. 
http://cr.openjdk.java.net/~rprotacio/8169734.01/

Rachel


On 12/2/2016 4:18 PM, Daniel D. Daugherty wrote:
> > I'd suggest to collect at least one more opinion/review on this.
>
> Let's get this correct now and not have to come back and fix it down
> the road. Looks to me like these new locations can take advantage
> of C++ string concatenation unless I'm missing something, e.g.:
>
> 775              "Incorrect boot loader search path, no java runtime 
> image or java.base exploded build");
>
> becomes:
>
> 775              "Incorrect boot loader search path, no java runtime 
> image or " JAVA_BASE " exploded build");
>
> which is a bit long, but not too hard to deal with... And since we're
> migrating away from an informal limit of 80 chars per line... I won't
> even complain... :-)
>
> Dan
>
>
> On 12/2/16 1:52 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Rachel,
>>
>>
>> On 12/2/16 11:12, Rachel Protacio wrote:
>>> Hi,
>>>
>>> Thanks for the review. Yeah, we had thought about that. Two 
>>> concerns: (1) the concatenation or %s is a little verbose and 
>>> possibly excessive for these ancillary error messages, and (2) 
>>> because they are error messages, potentially someone could hard-code 
>>> something to expect the output and if for some reason we change the 
>>> macro in the future it could throw people off unnecessarily... Just 
>>> some thoughts.
>>
>> Yes, I was thinking about the same.
>>
>>
>>> I'm happy to change all of them though if that's what we want.
>>
>> I'd suggest to collect at least one more opinion/review on this.
>> It is Ok with me to keep it as it is.
>>
>>
>>> I'll definitely change the classfile/vmSymbols.hpp and 
>>> runtime/os.cpp instances.
>>
>> Thanks!
>>
>> Thanks,
>> Serguei
>>
>>
>>> Let me know what you think!
>>>
>>> Rachel
>>>
>>> On 12/1/2016 5:43 PM, serguei.spitsyn at oracle.com wrote:
>>>> Hi Rachel,
>>>>
>>>> It looks good in general.
>>>> However, there are more instances of java.base that may need an 
>>>> update.
>>>>
>>>> classfile/classLoader.cpp:
>>>>
>>>>  775              "Incorrect boot loader search path, no java 
>>>> runtime image or java.base exploded build");
>>>>
>>>> 1826       vm_exit_during_initialization("Unable to create 
>>>> ModuleEntry for java.base");
>>>>
>>>>
>>>> classfile/modules.cpp:
>>>>
>>>>  180                 "Bad package name for module: java.base");
>>>>
>>>>  185                 err_msg("Invalid package name: %s for module: 
>>>> java.base", package_name));
>>>>
>>>>  191                 err_msg("Duplicate package name: %s for module 
>>>> java.base",
>>>>
>>>>  209   assert(ModuleEntryTable::javabase_moduleEntry() != NULL, "No 
>>>> ModuleEntry for java.base");
>>>>
>>>>  230           assert(pkg != NULL, "Unable to create a java.base 
>>>> package entry");
>>>>
>>>>  244               "Module java.base is already defined");
>>>>
>>>>  253   log_debug(modules)("define_javabase_module(): Definition of 
>>>> module: java.base,"
>>>>
>>>>  261     log_trace(modules)("define_javabase_module(): creation of 
>>>> package %s for module java.base",
>>>>
>>>>  716   assert(ModuleEntryTable::javabase_defined(), "Attempt to 
>>>> call get_module before java.base is defined");
>>>>
>>>>  762          "Attempt to call get_module_from_pkg before java.base 
>>>> is defined");
>>>>
>>>>  799          "Attempt to call get_named_module before java.base is 
>>>> defined");
>>>>
>>>>
>>>> runtime/arguments.cpp:
>>>>
>>>> 3432       vm_exit_during_initialization("Cannot specify java.base 
>>>> more than once to --patch-module");
>>>>
>>>>
>>>> Some other files have it too:
>>>>
>>>> classfile/vmSymbols.hpp: template(java_base, 
>>>> "java.base")                                \
>>>> classfile/moduleEntry.cpp:    fatal("Unable to finalize module 
>>>> definition for java.base");
>>>> classfile/moduleEntry.cpp:  assert(jb_module != NULL, "java.base 
>>>> ModuleEntry not defined");
>>>> classfile/moduleEntry.cpp:    fatal("Unable to patch the module 
>>>> field of classes loaded prior to java.base's definition, invalid 
>>>> java.lang.reflect.Module");
>>>> classfile/packageEntry.cpp: vm_exit_during_initialization("A 
>>>> non-java.base package was loaded prior to module system 
>>>> initialization", entry->name()->as_C_string());
>>>> oops/arrayKlass.cpp:         "module entry not available post 
>>>> java.base definition");
>>>> oops/instanceKlass.cpp: 
>>>> assert(ModuleEntryTable::javabase_moduleEntry() != NULL, "java.base 
>>>> module is NULL");
>>>> runtime/os.cpp:  char* base_classes = 
>>>> format_boot_path("%/modules/java.base", home, home_len, fileSep, 
>>>> pathSep);
>>>>
>>>> So, the question is if it's worth to update the remaining instances 
>>>> as well.
>>>> Simple concatenation or %s replacement in some rare cases could be 
>>>> used for it.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 12/1/16 13:20, Rachel Protacio wrote:
>>>>> Hi!
>>>>>
>>>>> Please review this little change, replacing errant uses of the 
>>>>> string "java.base" with the macro JAVA_BASE_NAME. Passes JPRT.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8169734
>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8169734.00/
>>>>>
>>>>> Thanks,
>>>>> Rachel
>>>>
>>>
>>
>>
>