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

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Fri Dec 2 20:52:16 UTC 2016


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



More information about the hotspot-runtime-dev mailing list