Please use a consistent name space for exported symbols

[mbiebl]

Please consider using a consistent name space for the exported symbols, i.e ABI and the API

currently exported symbols which don't use json_:
array_list_add@Base 0.99.2
array_list_bsearch@Base 0.99.2
array_list_free@Base 0.99.2
array_list_get_idx@Base 0.99.2
array_list_length@Base 0.99.2
array_list_new@Base 0.99.2
array_list_put_idx@Base 0.99.2
array_list_sort@Base 0.99.2
lh_abort@Base 0.99.2
lh_char_equal@Base 0.99.2
lh_kchar_table_new@Base 0.99.2
lh_kptr_table_new@Base 0.99.2
lh_ptr_equal@Base 0.99.2
lh_table_delete@Base 0.99.2
lh_table_delete_entry@Base 0.99.2
lh_table_free@Base 0.99.2
lh_table_insert@Base 0.99.2
lh_table_insert_w_hash@Base 0.99.2
lh_table_length@Base 0.99.2
lh_table_lookup@Base 0.99.2
lh_table_lookup_entry@Base 0.99.2
lh_table_lookup_entry_w_hash@Base 0.99.2
lh_table_lookup_ex@Base 0.99.2
lh_table_new@Base 0.99.2
lh_table_resize@Base 0.99.2
mc_debug@Base 0.99.2
mc_error@Base 0.99.2
mc_get_debug@Base 0.99.2
mc_info@Base 0.99.2
mc_set_debug@Base 0.99.2
mc_set_syslog@Base 0.99.2
printbuf_free@Base 0.99.2
printbuf_memappend@Base 0.99.2
printbuf_memappend_char@Base 0.99.2
printbuf_memappend_no_nul@Base 0.99.2
printbuf_memset@Base 0.99.2
printbuf_new@Base 0.99.2
printbuf_reset@Base 0.99.2
printbuf_terminate_string@Base 0.99.2
sprintbuf@Base 0.99.2

[mbiebl]

Or don't export that API/ABI at all, if not necessary

[rgerhards]

The problem here is that this stems back to json-c and json-c even considers most of them as official API. I agree this is unfortunate. We are thinking on how to handle this for our 1.0 release. The current state of mind is to not put too much pressure on keeping compatible with json-c.

[mbiebl]

this really opens a can of worms. If for some reason libjson-c and libfastjson are loaded into the same address space (e.g. foo links libfastjson and libbar and libbar links against libjson-c), then boom, subtle errors and crashes can be the result. As you can no longer predict which symbol is used by the binary.
That's pretty bad

[rgerhards]

For more on this discussion, see also http://lists.adiscon.net/pipermail/rsyslog/2016-April/042422.html

Bottom line: the name space needs to change for libfastjson because @mbiebl is totally right and I am wrong.

[mbiebl]

Ok, if we are going to use a distinct namespace after all and don't worrry about ABI (and API) compatibility with json-c, then I would use this opportunity and clean up the list of exported symbols and trim all internal symbols and use a consistent prefix (like fjson_ as you suggested on the m-l) for all symbols.

[rgerhards]

full ack! I think most of the things that do not start with json_ in json-c are actually internal-use functions and not API.

What would you suggest is the best way to hide these symbols? I was thinking about using __attribute__ ((visibility ("hidden"))) - does that sound like a good thing to you (I assume clang also supports it, but have not yet checked).

[mbiebl]

You can use that, or
a/ a symver file like in [1] or
b/ with libtool you can use -export-symbols-regex like in [2]

b/ is pretty convenient. A common scheme is to name the public symbols foo_ and the private ones foo
a/ gives you more control and avoids exporting symbols "by accident", because you forgot to add the preceding _

[1] https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm/libnm.ver
https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm/Makefile.am#n148
[2] https://cgit.freedesktop.org/upower/tree/libupower-glib/Makefile.am#n53

[rgerhards]

cool, thx. Will see what makes most sense in this context. I tend towards b/

[rgerhards]

@mbiebl I have just placed a PR. To me, it looks like now only "fjson_*" is being exported, but I would appreciate if you could have a quick check as well. I used nm --extern-only --defined-only for checking.

[mbiebl]

@rgerhards The ABI side looks fine now, only symbols with fjson_ are exported now.
The API side still needs fixing: After a make install, /usr/include/libfastjson/ contains

arraylist.h
bits.h
debug.h
json_config.h
json_c_version.h
json.h
json_inttypes.h
json_object.h
json_object_iterator.h
json_object_private.h
json_tokener.h
json_util.h
linkhash.h
math_compat.h
printbuf.h
random_seed.h

This needs some serious cleanup. Quite a few of those file should not be installed (and made sure, the other headers don't reference them anymore).

We probably also want to rename the json_ files to fjson_?

[mbiebl]

fjson_c_version() → fjson_version() and the corresponding header file renamed
json_c_version.[ch] → fjson_version.[ch]

[mbiebl]

fjson_c_get_random_seed() → fjson_get_random_seed() ?

[mbiebl]

So I guess we are not quite there yet. Should we reopen this issue or track this elsewhere?

[mbiebl]

Files which should not be part of the API and therefor not be installed in a public dir are probably at least the following:

arraylist.h
bits.h
debug.h
linkhash.h
math_compat.h → this file might be dropped altogether?
printbuf.h

The following file should be renamed
random_seed.h → (f)json_random_seed.h

[rgerhards]

We need to track this elsewhere. The reason is that this spawns multiple issues. The API will be changing until we hit 1.0.0. There are many aspects I would like to change, for example #35. My plan is to have interim releases (0.99.x) which clean up/change parts of the API until we reach 1.0.0, which will have the "final" set that we than guarantee to stay). Please let me know if you have concerns with this plan.

I have opened an issue tracker for the include files (#34) and there already is an issue tracker for the version files (#29).