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

[Daniel D. Daugherty]

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