-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Description
Describe the bug
In order to properly set symbol visibility, zstd uses some macros ZSTDLIB_API, ZDICTLIB_API and ZSTDERRORLIB_API.
Those get their values through some other macros:
ZSTDLIB_APIgets its value fromZSTDLIB_VISIBLE,ZDICTLIB_APIfromZDICTLIB_VISIBILITYandZSTDERRORLIB_APIfromZSTDERRORLIB_VISIBILITY.
As you can see, the naming is not consistent, one macro ends in the adjective VISIBLE while the other two end in the noun VISIBILITY.
This alone is confusing and might lead to errors. (I tripped over this when I pre-defined the macro ZSTDLIB_VISIBILITY, similar to the macros ZDICTLIB_VISIBILITY and ZSTDERRORLIB_VISIBILITY, in order to modify symbol visibility.)
And in fact, you seem to have run into this mistake, too: The Makefile in contrib/linux-kernel/Makefile makes exactly the same error and tries to replace the first occurrence of ZSTDLIB_VISIBILITY which does not exist.
A clear and concise description of what the bug is.
The bug is an inconsistency in naming that might lead to errors and which already lead to an error in the provided contrib/linux-kernel/Makefile.
Expected behavior/fix
All occurrences of ZSTDLIB_VISIBLE in the code should probably be replaced by ZSTDLIB_VISIBILITY.
Alternatively, the occurrence of ZSTDLIB_VISIBILITY in contrib/linux-kernel/Makefile needs to be replaced by ZSTDLIB_VISIBLE.
Screenshots and charts
N/A
Desktop (please complete the following information)
Not really relevant, but here you go:
- OS: Ubuntu Linux
- Version: 20.04
- Compiler: clang/gcc
- Flags: N/A
- Other relevant hardware specs: N/A
- Build system: CMake
Additional context
As explained above, this is especially important if one wants to modify symbol visibility.