On 11/29/2017 2:52 PM, Ferenc Wágner wrote:
"Fabio M. Di Nitto" fabbione@fabbione.net writes:
On 11/29/2017 8:50 AM, Fabio M. Di Nitto wrote:
On 11/28/2017 4:15 PM, Fabio M. Di Nitto wrote:
- load_compress_lib and load_crypto_lib should be able to stay inside compress.c and crypto.c. I don´t think there is a need to move them in common.c
this is done in the current module branch.
Hi Fabio,
Thanks for fixing this up. I parked them in common.c in the hope of future unification. The only algorithmic difference was the extra initialization call in load_crypto_lib(), which I really didn't understand in the light of the already existing init method, but left alone for the time. Now I see you already got rid of it, which is great.
Going further, the copying of the individual pointers could be replaced by defining pure "ops" structures in the modules, and only copying their single addresses into the respective ops pointers of the model structures. The rest is just string differences as far as I remembe
sounds about right.
There is also the lz4/lz4hc duality. Those models share a decompress method, which is currently duplicated in the two modules. It's no big deal because that code is tiny, but in the future it might be useful to contain the model name in the name of the exported symbol, optionally decoupling it from the module soname.
I have been thinking about it too but I don´t think it´s too urgent.
- we need to define something like #define KNET_INTERNAL_CRYPTO_API_VER so that we can use that either to identify the path on disk where to load the modules (ex: pkglibdir/crypto/$VER/ or $VER/crypto) or checked at load time (or both). this should address point 4 below.
For now I added an internal API version check. I am not sure yet we need to add also versioning on disk. It might be overkilling. What do you think?
I think module directory versioning would be better, as it allows coexistence of different module ABIs. That can help in your upgrade scenario, if the (already loaded and running) core library is upgraded and the modules with the old ABI version are left installed.
libknet1-1.0-x has internal API 1 libknet1-1.0-y has internal API 2
on update, the old dir would go away no matter what, unless you want to do metapackages similar to kernel and keep them around. IMHO, not worth it.
Speaking about the version check: is it necessary to store the version in the model description? It's a generic constant after all.
It is the easiest way to compare them and it´s only a uint8_t. Hardly a resource issue ;)
I tested doing build with version 1, copy the modules, do a version 2 build, overwrite the modules, etc.. each module needs to know its own version.
- log_msg should not be exported in the public API. This is a no-no from me ;) Either pass the pointer to the function to the init of a given module, or let´s find another way to report errors back from the modules to main libknet.
this is done in the current module branch.
Similarly to the above, is the function pointer worth storing in every handle? I understand it's handy, because it's being passed around anyway. My idea was writing it into another module symbol at load time and defining the logging macros to use that when compiling modules. Probably doesn't matter much, though.
I have tried to write it into a module symbol, but there was a remapping issue that had to be done per module. This one works across the board without per module changes. It´s a minor technicality IMHO.
I hope to get back to coding instead of theorizing soon... and I've just subscribed to devel@l.k.o to facilitate email communication. :)
Cool.. no worries. take your time.
Fabio