AppImageCrafters / AppRun

AppDir runtime components
MIT License
26 stars 10 forks source link

Fix #51 #52

Closed TheBrokenRail closed 2 years ago

TheBrokenRail commented 2 years ago

My reasoning for making it a global variable was that if something else modifies LD_PRELOAD, it would still keep track of chdir calls and handle appropriately even if libapprun_hooks.so isn't the first thing loaded. I'll definitely change apprun_restore_workdir_if_needed to run in a constructor function instead of the main hook though.

azubieta commented 2 years ago

If LD_PRELOAD is changed they can hook things before ours and change the working directory by the must restore it otherwise the app may fail. I guess that it's they responsibility to fix things.

Let's divide this PR in two:

TheBrokenRail commented 2 years ago

If LD_PRELOAD is changed they can hook things before ours and change the working directory by the must restore it otherwise the app may fail. I guess that it's they responsibility to fix things.

Let's divide this PR in two:

* One replacing the main function hook with the constructor

* Other to do the chdir modification (there we could continue the discussion)

The only other things done in the main hook is debug logging that main has been called, so I don't think it needs to be switched completely into a constructor function, only the chdir stuff. For now, I'll just get rid of the global variable.

azubieta commented 2 years ago

Seems good! Thanks !