c-blosc2 `plugins/plugin_utils.h` and `include/blosc2/plugins-utils.h`

Describe the bug

Two headers files have almost the same name, that doesn't feel right:

Expected behavior

Different header files should not share names that similar.

Additional context

What was the initial intent?

include/blosc2/plugins-utils.h mostly defines inline function load_lib, which is currently used only in the core library blocs2/blosc.c
plugins/plugin_utils.h declares function deserialize_meta which is used in plugins.

Perhaps merge the code either in plugins/plugin_utils.h or in include/blosc2/plugins-utils.h?

Of course, both header files can be kept for compatibility, we just need to decide which header file gets all the code and which header file remains empty (just including the other header).

Actually, where exactly does plugins/plugin_utils.c belong? As far as I can see, it's not linked into the library.

May 18 '23 14:05 DimitriPapadopoulos

Note that you have 4 files containing an underscore _:

$ find -path ./internal-complibs -prune -o -name '*_*.h' -print
./blosc/b2nd_utils.h
./plugins/plugin_utils.h
./tests/test_common.h
./tests/b2nd/test_common.h
$

and 23 files that contain a dash -:

$ find -path ./internal-complibs -prune -o -name '*-*.h' -print
./blosc/shuffle-avx2.h
./blosc/bitshuffle-sse2.h
./blosc/bitshuffle-generic.h
./blosc/shuffle-generic.h
./blosc/bitshuffle-altivec.h
./blosc/shuffle-neon.h
./blosc/trunc-prec.h
./blosc/bitshuffle-avx2.h
./blosc/shuffle-sse2.h
./blosc/transpose-altivec.h
./blosc/bitshuffle-neon.h
./blosc/blosc-private.h
./blosc/shuffle-altivec.h
./plugins/codecs/zfp/blosc2-zfp.h
./plugins/codecs/zfp/zfp-private.h
./plugins/codecs/ndlz/ndlz-private.h
./include/blosc2/blosc2-common.h
./include/blosc2/tuners-registry.h
./include/blosc2/filters-registry.h
./include/blosc2/plugins-utils.h
./include/blosc2/blosc2-stdio.h
./include/blosc2/blosc2-export.h
./include/blosc2/codecs-registry.h
$

How about standardising the file names (probably using -)?

May 18 '23 14:05 DimitriPapadopoulos

Describe the bug

Two headers files have almost the same name, that doesn't feel right:

include/blosc2/plugins-utils.h

plugins/plugin_utils.h

Expected behavior

Different header files should not share names that similar.

Additional context

What was the initial intent?

include/blosc2/plugins-utils.h mostly defines inline function load_lib, which is currently used only in the core library blocs2/blosc.c

plugins/plugin_utils.h declares function deserialize_meta which is used in plugins.

Perhaps merge the code either in plugins/plugin_utils.h or in include/blosc2/plugins-utils.h?

Yeah, this code has grown organically during the past months, and perhaps now is a good time to step back for some perspective and do this merge. If you can propose a PR we can discuss it more.

Of course, both header files can be kept for compatibility, we just need to decide which header file gets all the code and which header file remains empty (just including the other header).

Actually, where exactly does plugins/plugin_utils.c belong? As far as I can see, it's not linked into the library.

Apparently it is actually meant to just host deserialize_meta, which could be moved into a header (like include/blosc2/plugins-utils.h and make it available in devel packages).

May 18 '23 15:05 FrancescAlted

Ping?

Jun 07 '24 16:06 FrancescAlted

Meanwhile, include/blosc2/plugins-utils.h disappeared in 092e30d3.

Jun 10 '24 06:06 DimitriPapadopoulos

About deserialize_meta in plugins/plugin_utils.h / plugins/plugin_utils.c:

There's an inline b2nd_deserialize_meta function in include/b2nd.h, which is almost identical to deserialize_meta. It is currently used in the library only, but its documentation states:

* @note This function is inlined and available even when not linking with libblosc2.
Generally speaking, I have a hard time understanding the intended linkage of such functions. It looks like inline is (mis)used to avoid thinking how to link the function. Is b2nd_deserialize_meta supposed to be used as a static function within each file that uses it, bloating the library? Is inline useful in any way here? Is b2nd_deserialize_meta to be available outside the library? Is deserialize_meta supposed to be used as a static function within each plugin that uses it? Why not link it to the blosc library, making it available as a service to plugins within the API?

Jun 10 '24 06:06 DimitriPapadopoulos

The deserialize_meta function in plugin_utils was added when caterva project was still not merged in c-blosc2. Now that it has already been merged, we removed this function as b2nd_deserialize_meta can do the same . See 0157208e915933bcf027aff5759abe5a2bd6e705. Thanks for detecting this!

Jun 11 '24 09:06 martaiborra