Skip to content

Cleanup ObjectCheck - #20467

Merged
amicic merged 1 commit into
eclipse-openj9:masterfrom
dmitripivkine:master
Oct 31, 2024
Merged

Cleanup ObjectCheck#20467
amicic merged 1 commit into
eclipse-openj9:masterfrom
dmitripivkine:master

Conversation

@dmitripivkine

Copy link
Copy Markdown
Contributor

Update ObjectCheck.cpp and ObjecrCheck.hpp according Coding Standards:

  • use correct variable types
  • fix code formatting
  • replace extensions to ext

Update ObjectCheck.cpp and ObjecrCheck.hpp according Coding Standards:
- use correct variable types
- fix code formatting
- replace extensions to ext

Signed-off-by: Dmitri Pivkine <Dmitri_Pivkine@ca.ibm.com>
@dmitripivkine

Copy link
Copy Markdown
Contributor Author

fixes #20468


/* check alignment */
if (0 != ((UDATA)ptr & (sizeof(UDATA) - 1))) {
if (0 != ((uintptr_t)ptr & (sizeof(uintptr_t) - 1))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

while we can keep this change minimalistic (formatting only), we should probably use a proper const/variable rather then sizeof(uintptr_t)

we objectAlignmentInBytes available at a few places

although I'm a bit confused - ptr is pointer to RAM class, should we not have a strong check for 256 byte (for what we have J9_REQUIRED_CLASS_ALIGNMENT)!?

@dmitripivkine dmitripivkine Oct 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it caught my eye too, we need to check correctness as well is this code in use or obsolete. I added this to my todo list. I think it is better to keep this PR formatting only.
Regarding sizeof(uintptr_t) I thought about this too. Do we have a better declaration?

@amicic

amicic commented Oct 31, 2024

Copy link
Copy Markdown
Contributor

jenkins compile win,aix jdk21

@amicic
amicic merged commit 25b1266 into eclipse-openj9:master Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants