On 11/29/2017 2:52 PM, Ferenc Wágner wrote:
"Fabio M. Di Nitto"
<fabbione(a)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:
>
>> 1) 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.
> 2) 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.
>> 3) 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(a)l.k.o to facilitate email communication. :)
Cool.. no worries. take your time.
Fabio