libnxz / power-gzip

POWER NX zlib compliant library
23 stars 18 forks source link

Update exported symbols and add ABI check #116

Closed mscastanho closed 2 years ago

mscastanho commented 2 years ago

libnxz is currently exporting many symbols that are of internal use, mainly global variables. With the code in this PR all such symbols have been made internal, either by declaring them static, or moving into nx_config or even turning into #defines. There are still a few cases in which I'm not sure if any of the previous options apply, so I explicitly added them to the list of local symbols in the linker script.

Now we're down from 22 exported variables to 0.

I also included a test using abidiff to check ABI compatibility with zlib and to guarantee we spot ABI changes when they happen. A good way to test it is to remove one function from the list of missing_symbols in test/test_abi and run it again. Also, try to remove the static keyword from, for example, timeout_seconds in lib/gzip_vas.c. Both of these should demonstrate how the test will catch ABI changes.

There are some other misc changes as well, but I tried to separate each in their own commit to facilitate reviewing.

Closes #25

tuliom commented 2 years ago

In the 7th commit you added:

Make all nx_* symbols private

What does that mean? There are many nx_* symbols that are still global and they should continue global.

mscastanho commented 2 years ago

What does that mean? There are many nx_* symbols that are still global and they should continue global.

@tuliom That's bad wording on my part. I meant to say to group all local nx_* into a single wildcard statement in the linker script, instead of one for each. All global nx_* symbols are explicitly listed and will continue to be global. I'll update the commit message.

mscastanho commented 2 years ago

@tuliom I believe the latest push addresses all your comments. I just couldn't find a simple way to get the path to the system's libz.so to help regenerating libz.abi automatically, but I added instructions in the form of message to the user.

mscastanho commented 2 years ago

@tuliom I added the latest changes we discussed and also rebased on top of develop.