aix: guard STATIC_ASSERT for glibc work around#1808
aix: guard STATIC_ASSERT for glibc work around#1808richardlau wants to merge 1 commit intolibuv:v1.xfrom
Conversation
|
Until #1807 and nodejs/build#1245 are resolved the libuv CI doesn't actually build/test 64-bit AIX. I guess we could cherry pick this over to nodejs/node#20129 and test on the Node.js CI? |
|
If I understand it correctly, the wrapper structure |
| unsigned int value; | ||
| } uv_semaphore_t; | ||
|
|
||
| #ifdef __GLIBC__ |
There was a problem hiding this comment.
Looking at the changes that were made above, I think we also need to check for __MVS__ now, where we always seem to use this pattern
There was a problem hiding this comment.
I'd guard it on #if platform_needs_custom_semaphore. Can you drop the empty lines around the STATIC_ASSERT?
|
@gireeshpunathil Practically speaking, right now the assertion would go off only on 64-bit platforms where |
|
@addaleax - sorry, but I didn't follow you - the premise of the assertion is to make sure that the user's pointer (actual sem *) is capable of holding your new pointer right? so it is always pointer-to-pointer only, (never pointer-to-value) that means we don't need it, in any platforms, 32 or 64? Or Am I missing anything? |
If I’m understanding you correctly, no, that’s not the case. The static assertion is correct the way it is – we want to check that the |
|
got it, thanks! |
|
LGTM |
On 64-bit AIX `sizeof(uv_sem_t)` is 4 bytes which is not large enough to store a pointer. AIX doesn't use glibc so the work around introduced by libuv#1795 doesn't apply, so guard the STATIC_ASSERT so that it is only used when the custom semaphore implementation is used.
d12c51b to
5c98d6c
Compare
|
Empty lines around the STATIC_ASSERT have been dropped. Guard is now |
On 64-bit AIX `sizeof(uv_sem_t)` is 4 bytes which is not large enough to store a pointer. AIX doesn't use glibc so the work around introduced by #1795 doesn't apply, so guard the STATIC_ASSERT so that it is only used when the custom semaphore implementation is used. Refs: nodejs/node#20129 Refs: #1795 PR-URL: #1808 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Another PR to verify AIX - https://ci.nodejs.org/view/libuv/job/libuv-test-commit/801/ Landed in 56220e5. Thanks! |
Refs: nodejs/node#20129 (comment)
On 64-bit AIX
sizeof(uv_sem_t)is 4 bytes which is not large enough tostore a pointer. AIX doesn't use glibc so the work around introduced by
#1795 doesn't apply, so guard the
STATIC_ASSERT with
#ifdef __GLIBC__.cc @addaleax @cjihrig