COVESA / dlt-daemon

Diagnostic Log and Trace.
https://covesa.github.io/dlt-daemon/
Mozilla Public License 2.0
383 stars 297 forks source link

libdlt symbol hygiene #582

Open Felix-El opened 1 year ago

Felix-El commented 1 year ago

Today, libdlt exposes all of its symbols, which can lead to symbol conflicts. In fact, this is what brought me here. I was lucky my linker discovered some of such conflicts (many linkers won't). There is high risk code will silently break in unobvious ways and cause major debugging pain. This is similar to C's ODR (one definition rule) which causes UB when violated - at inter-project scale (exe + shared libs)! To reduce the "problem surface" libdlt should not contribute symbols like "buffer" to the host applications but only the public API.

I would like to contribute a patch that fixes the situation in the following way, if the community agrees (please comment).

set_target_properties(dlt PROPERTIES
    C_VISIBILITY_PRESET hidden
    CXX_VISIBILITY_PRESET hidden
    VISIBILITY_INLINES_HIDDEN 1
)

to stop exporting all symbols and utilize

include(GenerateExportHeader)
generate_export_header(dlt)

to have a header generated which offers the DLT_EXPORT macro to put in front of all public APIs like

DLT_EXPORT DltReturnValue dlt_init();

It appears that GenerateExportHeader was targetting only CXX apps before CMake 3.12. Would it be okay to up the minimum CMake 3.12? According ubuntu.com mirrors this version is available for Ubuntu 16.04.

minminlittleshrimp commented 1 year ago

Thank you @Felix-El , sound interesting to me. It worths investigating, I will look into it and response soon.