Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix string literal conversion warnings in env #18467

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Fix string literal conversion warnings in env #18467

merged 1 commit into from
Dec 8, 2023

Conversation

dylanjtuttle
Copy link
Contributor

@dylanjtuttle dylanjtuttle commented Nov 16, 2023 •
edited
Loading

Work towards fixing AIX warnings about assigning string literals to non-const char pointers by adding 'const' qualifiers to some string variables and parameters (or worst case scenario, casting to (char *)) in runtime/compiler/env.

This PR contributes to (but does not close) #14859

This PR depends on:

and must be merged in coordination with eclipse-omr/omr#7186

This was referenced Nov 16, 2023
Merged
Merged
Merged
Merged
@hzongaro hzongaro added the depends:omr Pull request is dependent on a corresponding change in OMR label Nov 16, 2023
@dylanjtuttle dylanjtuttle marked this pull request as draft November 16, 2023 20:20
@dylanjtuttle dylanjtuttle changed the title Fix -Wwritable-strings in env Fix string literal conversion warnings in env Nov 22, 2023
@dylanjtuttle dylanjtuttle marked this pull request as ready for review November 23, 2023 18:17
@hzongaro hzongaro self-assigned this Nov 29, 2023
hzongaro
hzongaro reviewed Nov 29, 2023
View reviewed changes
Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice there are references to getMethodFromName and getMethodFromClass in JITClientCompilationThread.cpp that use const_cast<char *>. Those const_casts can be removed now that the various getMethodFromName and getMethodFromClass methods' arguments have changed to const char *.

runtime/compiler/env/CHTable.hpp Outdated Show resolved Hide resolved
runtime/compiler/env/VMJ9.cpp Show resolved Hide resolved
runtime/compiler/env/annotations/AnnotationBase.hpp Outdated Show resolved Hide resolved
runtime/compiler/env/annotations/Annotations.cpp Outdated Show resolved Hide resolved
runtime/compiler/env/annotations/Annotations.cpp Outdated Show resolved Hide resolved
runtime/compiler/env/j9method.cpp Show resolved Hide resolved
runtime/compiler/env/j9method.cpp Outdated Show resolved Hide resolved
runtime/compiler/env/j9method.cpp Show resolved Hide resolved
hzongaro
hzongaro approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks! May I ask you to squash the two commits?

@hzongaro
Copy link
Member

Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk11,jdk17,jdk21 depends eclipse-omr/omr#7186

hzongaro
hzongaro approved these changes Dec 4, 2023
View reviewed changes
Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the latest revision!

@hzongaro
Copy link
Member

hzongaro commented Dec 4, 2023

Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk11,jdk17,jdk21 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 5, 2023

pLinux JDK21 build failure does not appear to be related to this change. Rerunning.

14:49:12  IOException caught during compilation: Server failed to initialize: No port file values materialized. Giving up after 60099 ms
14:49:12  gmake[3]: *** [/home/jenkins/workspace/Build_JDK21_ppc64le_linux_Personal/build/linux-ppc64le-server-release/jdk/modules/java.se/_the.java.se_batch] Error 1
14:49:12  CompileJavaModules.gmk:94: recipe for target '/home/jenkins/workspace/Build_JDK21_ppc64le_linux_Personal/build/linux-ppc64le-server-release/jdk/modules/java.se/_the.java.se_batch' failed
14:49:12  make/Main.gmk:197: recipe for target 'java.se-java' failed
14:49:12  gmake[2]: *** [java.se-java] Error 1

Jenkins test sanity.functional,sanity.openjdk plinux jdk21 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 5, 2023

Jenkins test sanity.opendjk win jdk17 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 6, 2023

Jenkins test sanity.opendjk win jdk17 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 6, 2023

Windows sanity.openjdk JDK 17 run failed again. It looked like it was an infrastructure problem. All other test runs were successful or failed due to known issues. The most recent change only affects z/OS builds, so rerunning the Windows sanity.openjdk JDK 17 should be sufficient. I'll try out z/OS internally.

Jenkins test sanity.opendjk win jdk17 depends eclipse-omr/omr#7186

@dylanjtuttle
Fix string literal conversion warnings in runtime/compiler/env
121b33e 
Fix string literal conversion warnings in runtime/compiler/env

Signed-off-by: Dylan Tuttle <[email protected]>
@hzongaro
Copy link
Member

hzongaro commented Dec 7, 2023

Giving the Windows testing one more try:

Jenkins test sanity.opendjk win jdk17 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 7, 2023

Argh! Typo in my build requests.

Jenkins test sanity.openjdk win jdk17 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 7, 2023

Jenkins test sanity.openjdk win jdk17 depends eclipse-omr/omr#7186

@hzongaro
Copy link
Member

hzongaro commented Dec 8, 2023

Windows sanity.openjdk JDK17 testing was successful this time. Testing internally on z/OS and a variety of other platforms showed no unexpected failures. Going ahead with merging this pull request together with eclipse-omr/omr#7186.

@hzongaro hzongaro merged commit 21f742f into eclipse-openj9:master Dec 8, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzongaro hzongaro hzongaro approved these changes

@dsouzai dsouzai Awaiting requested review from dsouzai dsouzai is a code owner

Assignees

@hzongaro hzongaro

Labels
comp:jit depends:omr Pull request is dependent on a corresponding change in OMR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dylanjtuttle @hzongaro