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

_load_libc is not needed #776

Merged
merged 1 commit into from
May 4, 2021
Merged

Conversation

UnitedMarsupials
Copy link
Contributor

libc is used by python interpreter itself, thus dlopen(NULL) is sufficient -- we don't need to load the library again, and can instead refer to the functions already in the running process.

is sufficient -- we don't need to load the library again.
@UnitedMarsupials
Copy link
Contributor Author

Pretty sure, the test-failure is not due to the change I made...

@BoboTiG
Copy link
Collaborator

BoboTiG commented Apr 9, 2021

Hello @UnitedMarsupials,

How confident are you about such change given the amount of different GNU/Linux distributions out there? Maybe even some Android flavors are in the loop.

I did not know that trick, and looked up for more explanations. It seems a good one to use, but I am not 100% sure that will not break something somewhere for someone.

@UnitedMarsupials
Copy link
Contributor Author

The specification for dlopen states:

If file is a null pointer, dlopen() shall return a global symbol table handle for the currently running process image. This symbol table handle shall provide access to the symbols from an ordered set of executable object files consisting of the original program image file, any executable object files loaded at program start-up as specified by that process file (for example, shared libraries), and the set of executable object files loaded using dlopen() operations with the RTLD_GLOBAL flag. As the latter set of executable object files can change during execution, the set of symbols made available by this symbol table handle can also change dynamically.

So, yes, quite confident... Maybe, some OS out there is not compliant with the above, but inotify is Linux-only anyway...

@BoboTiG
Copy link
Collaborator

BoboTiG commented Apr 10, 2021

Let's go then. Could you add a comment above the code for history + a line in the changelog (add your nickname too in it)?

@BoboTiG BoboTiG merged commit 1415238 into gorakhargosh:master May 4, 2021
@BoboTiG
Copy link
Collaborator

BoboTiG commented May 4, 2021

Thanks @UnitedMarsupials :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants