libsc
libsc copied to clipboard
sc_puff should not be exported
Currently, the sc_puff
symbol is exported, even when it is completely unused (i.e. when using zlib).
- sc_puff.c should not be included in the library when using zlib
- sc_puff should never be exported from the shared library ("visibility hidden")
- The
sc_builtin/puff.h
header file should be removed from the sources (completely unused) - The
sc_puff.h
header file should not be installed, and probably moved to the sc_builtin directory
Thanks for your comment! I'm referring to the latest develop branch, where puff.h is removed.
We are already taking care that sc_puff is the only symbol exported, so the namespace is clean. What is wrong with always compiling and installing it? Does the function get in the way in any way?
Exported symbols should be kept at minimum. For most users it is just dead code, increasing the size of the binary - code section and symbol table.
Thanks again; there are two angles to this issue, as far as I can see.
- If sc_puff is just a fallback in case zlib is not found, it would be ok to never export it and only compile it as a replacement. In this case, I'm with you.
- If sc_puff is considered independently valuable, maybe to benchmark/test against zlib, or for any other purpose we're forgetting right here, the author of a library is free to make the choice to always compile. This is more where I am.
(There may be an in-between in that with zlib found, sc_puff itself may call zlib's uncompress function. I might go there but I also might not.)
So I'm willing to discuss any hard reasons that preclude sc_puff being standard. I'm not willing to discuss 1. vs. 2., if that's ok, since this is ultimately a matter of opinion and taste.
Thanks again for watching out and bringing up your concern!
Actually I'm playing with the feature-puff branch to test further suggestions of yours.
Actually I'm playing with the feature-puff branch to test further suggestions of yours.
So is this significantly better? See ac2f826cbab22072c90c63d0cc2a16561f6c3393..7966db0b60648d98f7907086b56c90475fec0508.
Hmm. With this change, sc_puff is an external symbol, which is compiled and supplied with a prototype only when zlib is not found. This makes the set of exported symbols dependent on the configuration, which may not be the best for consistency. Still leaning towards unconditionally compiling sc_puff. The question remains whether the prototype should be visible in installed .h files or not.