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.

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

internal/dlsym: Define _GNU_SOURCE for RTLD_DEFAULT #1026

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

anoopcs9
Copy link
Collaborator

man dlsym(3) says the following:

The _GNU_SOURCE feature test macro must be defined in order to obtain the definitions of RTLD_DEFAULT and RTLD_NEXT from <dlfcn.h>.

@anoopcs9 anoopcs9 force-pushed the gnu-source-rtld-default branch from e3c57a0 to 603f764 Compare September 13, 2024 12:48
@anoopcs9 anoopcs9 marked this pull request as ready for review September 13, 2024 13:12
@anoopcs9 anoopcs9 added extended-review A submitter or reviewer feels the PR needs an extended review period no-API This PR does not include any changes to the public API of a go-ceph package labels Sep 13, 2024
@ansiwen
Copy link
Collaborator

ansiwen commented Sep 13, 2024

Do we already use non-POSIX extensions elsewhere? Then I guess this shouldn't hurt. Otherwise the former approach is probably more portable. Not sure about Darwin+clang, if it emulates the GNU stuff.

@anoopcs9
Copy link
Collaborator Author

Do we already use non-POSIX extensions elsewhere? Then I guess this shouldn't hurt. Otherwise the former approach is probably more portable.

cephfs defines it but rbd explicitly undefines it. Provided that this change is internal I think we can use it and whoever(packages) doesn't want can undefine as we saw with rbd.

Not sure about Darwin+clang, if it emulates the GNU stuff.

I don't know.

@anoopcs9
Copy link
Collaborator Author

Not sure about Darwin+clang, if it emulates the GNU stuff.

I don't know.

clang should not be a problem as this macro is not tied to compiler but libc underneath.

Copy link
Member

@nixpanic nixpanic 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 to me, thanks!

@anoopcs9
Copy link
Collaborator Author

@ansiwen PTAL.

@anoopcs9
Copy link
Collaborator Author

@Mergifyio rebase

Copy link

mergify bot commented Sep 24, 2024

rebase

✅ Branch has been successfully rebased

@anoopcs9 anoopcs9 force-pushed the gnu-source-rtld-default branch from 603f764 to 403aad1 Compare September 24, 2024 17:10
@anoopcs9
Copy link
Collaborator Author

@Mergifyio rebase

man dlsym(3) says the following:
. . .
The _GNU_SOURCE feature test macro must be defined in order to obtain
the definitions of RTLD_DEFAULT and RTLD_NEXT from <dlfcn.h>.
. . .

Signed-off-by: Anoop C S <[email protected]>
Copy link

mergify bot commented Sep 25, 2024

rebase

✅ Branch has been successfully rebased

@anoopcs9 anoopcs9 force-pushed the gnu-source-rtld-default branch from 403aad1 to 83dac0d Compare September 25, 2024 13:26
@ansiwen
Copy link
Collaborator

ansiwen commented Sep 25, 2024

LGTM, manually checked on darwin.

@anoopcs9 anoopcs9 removed the extended-review A submitter or reviewer feels the PR needs an extended review period label Sep 25, 2024
@mergify mergify bot merged commit 065319c into ceph:master Sep 25, 2024
16 checks passed
@anoopcs9 anoopcs9 deleted the gnu-source-rtld-default branch September 25, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants