Closed jkroonza closed 3 years ago
Hello @jkroonza First of all, thank you so much for this PR - I always love to see how the community support us, thanks. I'm going to quickly check your PR and I'll merge it. I'm also open to discuss about other fixes - actually we already plan a big review of the code - so feel free to send a PR or make a discussion in order to improve this project.
Best Regards Michele
I'm also open to discuss about other fixes - actually we already plan a big review of the code - so feel free to send a PR or make a discussion in order to improve this project.
Hi Mechele,
I generally submit fixes as and when I find them, time to sort out all the issues was unfortunately at a premium, still need to finalize packaging, and the next one up might be constructs like --enable-ssl vs --disable-ssl (where --disable seems to not quite be the same as not specifying it at all ...). This I blame on autoconf not having a trinary structure whereby you specify the feature name and the test and then do something like:
if (--disable not given) {
if (test passes) {
enable feature
} else {
if (--enable given)
fail configure
disable feature
}
} else {
disable feature
}
I repeatedly have this issue in many projects where I either package or am involved with at some point or another.
Yes, I agree :( Anyway let me review the PR in order to merge. If you have suggestions or idea on how to improve the autoconf (or made some changes) we can also create a dedicated branch only for those kind of experiments.
Pull request successfully tested
This does not fix any of the other bugs I spotted along the way (module_path being assigned a static string and then later free()d for example).