-
Notifications
You must be signed in to change notification settings - Fork 728
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.
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
Changes to further OSX compilation #738
Conversation
runtime/port/sysvipc/j9shmem.c
Outdated
#if defined (__GNUC__) || defined (AIXPPC) | ||
#if defined (__GNUC__) | ||
#if defined(__GNUC__) || defined(AIXPPC) | ||
#if defined(__GNUC__) && !defined(OSX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be written as:
#if defined(OSX)
/*Use ._key for OSX*/
+ if (buf.shm_perm._key != controlinfo->common.ftok_key)
#elif defined(__GNUC__)
/*Use .__key for __GNUC__*/
if (buf.shm_perm.__key != controlinfo->common.ftok_key)
#elif defined(AIXPPC)
......
#endif
#if defined (__GNUC__) || defined (AIXPPC) || defined(J9ZTPF) | ||
#if defined (__GNUC__) && !defined(J9ZTPF) | ||
#if defined(__GNUC__) || defined(AIXPPC) || defined(J9ZTPF) | ||
#if defined(__GNUC__) && !defined(J9ZTPF) && !defined(OSX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a similar refactoring as suggested above be done here as well?
573fdbd
to
e798041
Compare
Refactoring performed. |
/*Use .key for AIXPPC*/ | ||
if (buf.sem_perm.key != controlinfo->ftok_key) | ||
#endif | ||
#if defined(J9ZTPF) | ||
#elif defined(J9ZTPF) | ||
/*Use .key for z/TPF */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix the comment spacing here since the files being modified anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, sorry I should've caught that.
@jdekonin Change looks good to me and I'll launch builds on it after the minor comment indenting issue is fixed |
Signed-off-by: Joe deKoning <[email protected]>
Signed-off-by: Joe deKoning <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Jenkins test sanity plinux |
The changes to j9porterror.h were originally from @DanHeidinga that I took over as per his agreement in #36. It needed a minor modification to compile on windows, an (I_32) addition.