RFR: 8259894: refactor parts of jvm.h into jvm_io.h and jvm_constants.h [v2]
Ioi Lam
iklam at openjdk.java.net
Thu Jan 21 03:09:17 UTC 2021
On Wed, 20 Jan 2021 05:51:11 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>> David Holmes comments
>
> src/hotspot/share/include/jvm_constants.h line 8:
>
>> 6: * under the terms of the GNU General Public License version 2 only, as
>> 7: * published by the Free Software Foundation. Oracle designates this
>> 8: * particular file as subject to the "Classpath" exception as provided
>
> Classpath exception is wrong here - that is only for (mainly Java) sources that applications may link against. It's existing use in jvm.h is incorrect.
I think the classpath exception is intended for other JVM implementations that might include jvm.h. I'll leave it as is for now.
> src/hotspot/share/include/jvm_constants.h line 26:
>
>> 24: */
>> 25:
>> 26: #ifndef _JAVASOFT_JVM_CONSTANTS_H_
>
> We don't need to keep the legacy JAVASOFT defines.
Changed to `_JVM_CONSTANTS_H_` and `_JVM_IO_H_`.
> src/hotspot/share/logging/logSelection.cpp line 25:
>
>> 23: */
>> 24: #include "precompiled.hpp"
>> 25: #include "jvm.h"
>
> Not jvm_io.h ??
Changed to jvm_io.h
> src/hotspot/share/include/jvm.h line 34:
>
>> 32: #include "jvm_md.h"
>> 33: #include "jvm_constants.h"
>> 34: #include "jvm_io.h"
>
> Note that the comment block starting at line 40 needs to be updated now you have refactored things.
I fixed the comments.
> src/hotspot/share/classfile/javaClasses.cpp line 26:
>
>> 24:
>> 25: #include "precompiled.hpp"
>> 26: #include "jvm.h"
>
> Does this really need jvm.h or just jvm_constants.h and jvm_io.h?
jvm.h is needed because this file uses the JVM_XXX() functions.
> src/hotspot/share/classfile/vmIntrinsics.cpp line 26:
>
>> 24:
>> 25: #include "precompiled.hpp"
>> 26: #include "jvm.h"
>
> Does this really need jvm.h or just jvm_constants.h and jvm_io.h?
Changed to jvm_constants.h and jvm_io.h.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2154
More information about the hotspot-dev
mailing list