Namespace internal stream_* functions #98
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Although (from what I could gather) these are not part of the public API, and meant to be only internally used, they still end up as text symbols in the resulting shared libraries. On the other hand, these names are generic enough that other libraries might also export symbols with the same name (which again is sub-optimal, as they should also namespace _their_ functions). This clash of names leads to situations where, depending on whether libcfitsio and libAnotherLibrary was loaded first (or at all) into memory or not, the stream_ function of one library might be invoked instead of the other, usually leading to segmentation faults. This commit renames the stream_* functions in libcfitsio to be prefixed with fits_. An alternative solution would be to hide these at link time, but that would require more widespread infrastructural changes. Note that although similar issues might occur with other functions with generic names (e.g., "file_close", "mem_init" and so on), this commit focuses only on the problem at hand. Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>
Contributor
Author
|
BTW, the library in particular that we were clashing against was zfp, for which I also opened a discussion item at llnl/zfp#279. In their case though the offending function is part of the public API, so namespacing their functions will be slow, if it ever happens. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Although (from what I could gather) these are not part of the public API, and meant to be only internally used, they still end up as text symbols in the resulting shared libraries. On the other hand, these names are generic enough that other libraries might also export symbols with the same name (which again is sub-optimal, as they should also namespace their functions). This clash of names leads to situations where, depending on whether libcfitsio and libAnotherLibrary was loaded first (or at all) into memory, the
stream_ functionof one library might be invoked instead of the other, usually leading to segmentation faults.This commit renames the stream_* functions in libcfitsio to be prefixed with fits_. An alternative solution would be to hide these at link time, but that would require more widespread infrastructural changes.
Note that although similar issues might occur with other functions with generic names (e.g., "file_close", "mem_init" and so on), this commit focuses only on the problem at hand.