search

facebook / zstd

Zstandard - Fast real-time compression algorithm
http://www.zstd.net
Other
23.58k stars 2.1k forks source link

Hide static symbols from dynamic library by default #1111

Open mestia opened 6 years ago

mestia commented 6 years ago

Hi, It seems that the following symbols are missing in 1.3.4:

Can be easily verified by comparing nm -D output nm -D /usr/lib/x86_64-linux-gnu/libzstd.so.1.3.3

Is that something you are aware of ? Thanks!

terrelln commented 6 years ago

These symbols are hidden behind ZSTD_STATIC_LINKING_ONLY, and don't have stability guarantees. They were renamed to be more consistent with the rest of the new advanced API. See https://github.com/facebook/zstd/blob/dev/lib/zstd.h#L1206.

mapreri commented 6 years ago

I'm sorry but I can't even seem to find them by grepping the whole sources, so they don't seem just "hidden" by being declared as static.

Nonetheless, if they were exported in the past, and now are not anymore, it's an ABI break without appropriate SONAME change. Was that intended?

terrelln commented 6 years ago

@mapreri, sorry I could've been more clear. These symbols were renamed from ZSTD_initCCtxParams() to ZSTD_CCtxParams_init() and such. They are part of the unstable API which doesn't maintain compatibility between versions (though we try not to silently break the code). To access the unstable API you have to define ZSTD_STATIC_LINKING_ONLY before including zstd.h, named because it isn't safe to use these symbols while dynamically linking in zstd.

mapreri commented 6 years ago

Oh, I see what you mean now.

Can't you do something to avoid having those symbols exported in the symbols table? (e.g. using the gcc support for visibility, or by properly teaching the linker what to do (guess the most complex option here would be to use a version script)).

That would actually prevent dynamic linked programs from accessing those functions, which is probably what you want. As it stands now, somebody who thinks to be a very clever programmer can just #define that const e use the functions even when dynamically linking.

terrelln commented 6 years ago

Yeah, we can provide a macro ZSTDLIB_STATIC_VISIBILITY that can be overridden if need be.

mestia commented 5 years ago

The following 6 symbols are missing in 1.3.8 compared to 1.3.5, are they removed due to the advanced API stabilization effort ? Thanks!

  1. ZSTD_setDStreamParameter
  2. ZSTD_decompress_generic_simpleArgs
  3. ZSTD_decompress_generic
  4. ZSTD_compress_generic
  5. ZSTD_compress_generic_simpleArgs
  6. ZSTD_CCtx_resetParameters
Cyan4973 commented 5 years ago

Yes, all these symbols are part of the (unstable) experimental API, and were updated to newer versions within the "almost stabilized" advanced API.

ZSTD_setDStreamParameter  --> ZSTD_DCtx_setParameter
ZSTD_decompress_generic_simpleArgs  --> ZSTD_decompressStream_simpleArgs
ZSTD_decompress_generic  --> ZSTD_decompressStream
ZSTD_compress_generic  --> ZSTD_compressStream2
ZSTD_compress_generic_simpleArgs  --> ZSTD_compressStream2_simpleArgs
ZSTD_CCtx_resetParameters  --> ZSTD_CCtx_reset

Symbols from the experimental API, although unstable, are still exposed in the dynamic library for the time being. This design choice could be revisited, either for v1.4.0 or v1.5.0.