jgaeddert / liquid-dsp

digital signal processing library for software-defined radios
http://liquidsdr.org
MIT License
1.87k stars 439 forks source link

exit() calls from shared library #134

Closed mdomsch closed 4 years ago

mdomsch commented 6 years ago

When packaging liquid-dsp for Fedora, rpmlint reports the following as a warning:

liquid-dsp.x86_64: W: shared-lib-calls-exit /usr/lib64/libliquid.so exit@GLIBC_2.2.5 This library package calls exit() or _exit(), probably in a non-fork() context. Doing so from a library is strongly discouraged - when a library function calls exit(), it prevents the calling program from handling the error, reporting it to the user, closing files properly, and cleaning up any state that the program has. It is preferred for the library to return an actual error code and let the calling program decide how to handle the situation.

jgaeddert commented 6 years ago

This has been brought up several times, and I wholeheartedly agree. It was a coding choice early on for handling errors, and was definitely a bad one. I do have a local branch where I am working on better error handling that doesn't involve calling exit(), but I have not yet merged it into master yet.

ncorgan commented 4 years ago

What's the status on this change? This has been a pain point in my development, as in some cases, I can't feasibly do the input validation necessary to avoid making liquid-dsp calls that result in an exit().

jgaeddert commented 4 years ago

Fair point. What I can do as a relatively quick solution is make exit calls only enabled at compile time with configure arguments, disabled by default. I agree this is a pain point.

jgaeddert commented 4 years ago

Well, this turned out to be a much larger effort than I originally anticipated, but I've removed all the exit() calls within the library with the exception of when you configure with --enable-strict which essentially restores this functionality (off by default). Most methods that didn't return, now return an integer error code in the grand C-style tradition.

mdomsch commented 3 years ago

@jgaeddert thank you for this labor of love. I found I needed to remove --enable-coverage to keep the exit() call from staying out of the final library, but that's fine - the Fedora package isn't using the coverage results anyhow.