Limit set of exported symbols

[Felix-El]

Patch for #582. This disables unconditional export of symbols from the shared library libdlt in favor of an opt-in approach.

• only symbols marked DLT_EXPORT are exposed from the shared library
• this commit adds DLT_EXPORT on all symbols declared in the installed header files
• static library builds of libdlt are unaffected by this change (BUILD_SHARED_LIBS=OFF)

# Build previous HEAD
$> nm --dynamic --defined-only ./build/src/lib/libdlt.so | cut -f3 -d' ' | sort > ./before
# Build this commit
$> nm --dynamic --defined-only ./build/src/lib/libdlt.so | cut -f3 -d' ' | sort > ./after
# Summary (Linux, with maximum options enable, -DDLT_IPC=UNIX_SOCKET)
$> wc -l ./before ./after
  331 ./before
  272 ./after

The sorted list of remaining symbols is:

$> cat ./after
dlt_buffer_check_size
... all prefixed with dlt ...
dlt_with_timestamp
get_filename_ext
getFileSerialNumber
multiple_files_buffer_file_name
... more multiple_files_buffer_* ...
multiple_files_buffer_write_chunk

So some functions need to be revised:

1. dlt_buffer_* defined in src/shared/dlt_common.c These functions are required to be exported but they are not declared in any header.
2. get_filename_ext declared in include/dlt/dlt_common.h It is not prefixed with dlt. Should it really be exported?
3. getFileSerialNumber defined in src/lib/dlt_filetransfer.c This function should be renamed to have a dlt prefix. It is also required to be exported but not declared in any header.
4. multiple_files_buffer_* declared in include/dlt/dlt_multiple_files.h These functions appear to be public API, too but are not prefixed with dlt.

[alexmohr]

At least 4. should not be used via public API imho as these functions are used internally to have rolling file names. There is no point in exposing that via the public API as clients should not write files with the dlt library. getFileSerialNumber is really necessary for public api? Shouldn't the public api just offer functions to transmit files w/o the user caring about how in detail the file is sent?

[Felix-El]

At least 4. should not be used via public API imho as these functions are used internally to have rolling file names. There is no point in exposing that via the public API as clients should not write files with the dlt library. getFileSerialNumber is really necessary for public api? Shouldn't the public api just offer functions to transmit files w/o the user caring about how in detail the file is sent?

The symbols I listed are not only public (user-facing) API. However, they have to be exported at the moment because some console tools expect them to be there. This is because libdlt is currently also used for sharing some code (implementation details) between daemon and other tools in the project.

Generally I'd propose to extract these details into a separate static library with private headers. To continue with this, however, I'd need some basic commitment to this PR's general idea by any one of the project responsible.

[michael-methner]

Hello @Felix-El , thanks for your contribution. I think this really make sense.

As this anyhow breaks ABI-compatiblity, I would also like to have the functionname #2 to #4 renamed. Removing the common code for dlt-daemon and dlt-tools I would keep for a separate PR.

There is a version bump in cmake. So DLT will not be compilable on older platforms (e.g. Ubuntu 18.04). Can you make this changes backward compatible? E.g. by using this feature only on recent cmake versions?

[Felix-El]

As this anyhow breaks ABI-compatiblity, I would also like to have the functionname #2 to #4 renamed.

Yus, totally agree.

Removing the common code for dlt-daemon and dlt-tools I would keep for a separate PR.

Fine for me.

There is a version bump in cmake. So DLT will not be compilable on older platforms (e.g. Ubuntu 18.04). Can you make this changes backward compatible? E.g. by using this feature only on recent cmake versions?

Is it maybe an option to just copy https://github.com/Kitware/CMake/blob/v3.12.0/Modules/GenerateExportHeader.cmake into this repo? License is BSD 3-Clause so should not be an issue. I would just need to verify this generates fine even with CMake as old as 3.3.

[Felix-El]

Hello everyone, I have just pushed an updated version which addresses above discussion points.

• get_filename_ext renamed to dlt_get_filename_ext
• getFileSerialNumber renamed to dlt_get_file_serial_number
• multiple_files_* were not renamed but excluded from export
• tested to build with CMake down to v3.5.1 on Ubuntu 16.04 (GenerateExportHeader copied from v3.12.0)

Since there are many more API to revise for "should they be public?" I deemed it enough to rename the few non-dlt* prefixed exported functions so we have everything under a dlt* "namespace" for the time being. This already reduces the risk of conflict extremely and further improvement can be done after merging this.

Reminder: Already the original commit removed up to ~60 symbols from the shared library's export table which were non-namespaced private implementation details. Only few symbols were under discussion.

[minminlittleshrimp]

Hello @Bichdao021195 Please kindly check this rework of dlt symbol export. Regards

[BichDaoBosch]

Hi @minminlittleshrimp I tested to build with CMake v3.3 on Ubuntu 20.

 Build previous HEAD
$> nm --dynamic --defined-only ./build/src/lib/libdlt.so | cut -f3 -d' ' | sort > ./before
# Build this commit
$> nm --dynamic --defined-only ./build/src/lib/libdlt.so | cut -f3 -d' ' | sort > ./after

$> wc -l ./before ./after
314 ./before
  258 ./after
  572 total

With cat ./after , i can observed that get_filename_ext renamed to dlt_get_filename_ext ->ok getFileSerialNumber renamed to dlt_get_file_serial_number -> ok multiple_files_* were not renamed but excluded from export -> ok tested to build with CMake v3.3 on Ubuntu 20 -> able to be exported Thanks Dao Nguyen

[Felix-El]

@michael-methner, @minminlittleshrimp how can we get this merged?