[Enhancement] Conditionally define MODBUS_EXPORTS only for shared library builds

[bravo-jcd]

[Enhancement] Conditionally define MODBUS_EXPORTS only for shared library builds

Introduction

First, I'd like to express my appreciation for ModbusLib - it's a well-designed and useful library! The build system works correctly for both static and shared library configurations, and the multi-layer approach with MB_DYNAMIC_LINKING is elegant.

This is a minor enhancement suggestion that could make the build configuration slightly clearer and more efficient, though the current implementation works perfectly fine.

Current Behavior

In src/CMakeLists.txt (line 217), MODBUS_EXPORTS is defined unconditionally for all builds:

add_library(${MB_LIBRARY_NAME} ${HEADERS} ${SOURCES} ${RESOURCES})
target_compile_definitions(${MB_LIBRARY_NAME} PRIVATE MODBUS_EXPORTS)

This means MODBUS_EXPORTS is defined even when building static libraries (BUILD_SHARED_LIBS=OFF).

Why Current Implementation Works Correctly

The library correctly handles this through the intelligent header design in ModbusGlobal.h:

#ifdef MB_DYNAMIC_LINKING
    // Only check MODBUS_EXPORTS if building shared library
    #if defined(MODBUS_EXPORTS) && defined(MB_DECL_EXPORT)
        #define MODBUS_EXPORT MB_DECL_EXPORT
    #elif defined(MB_DECL_IMPORT)
        #define MODBUS_EXPORT MB_DECL_IMPORT
    #else
        #define MODBUS_EXPORT
    #endif
#else
    // Static library - MODBUS_EXPORT is always empty
    #define MODBUS_EXPORT
#endif

The MB_DYNAMIC_LINKING check acts as the primary gate, so even though MODBUS_EXPORTS is defined during static builds, it's effectively ignored. This design works perfectly.

Suggested Enhancement

For improved clarity and semantic correctness, consider defining MODBUS_EXPORTS only when building shared libraries:

add_library(${MB_LIBRARY_NAME} ${HEADERS} ${SOURCES} ${RESOURCES})

if (BUILD_SHARED_LIBS)
    target_compile_definitions(${MB_LIBRARY_NAME} PRIVATE MODBUS_EXPORTS)
endif()

Benefits of This Change

1. Semantic Clarity: The preprocessor definition more clearly reflects the build type
2. Reduced Confusion: Developers examining build commands won't see -DMODBUS_EXPORTS when building static libraries
3. Slight Efficiency: One less preprocessor symbol to track (minor benefit)
4. Intent Communication: The CMake file more explicitly shows that MODBUS_EXPORTS is only relevant for DLL builds
5. Standard Practice: Aligns with common CMake patterns where export definitions are conditional

Impact Assessment

• Breaking Changes: None - this is purely an internal build change
• Behavioral Changes: None - output libraries are identical
• Risk: Minimal - the change is straightforward and localized
• Testing: Existing tests should pass without modification

Example From Build Output

Current static library build shows:

bcc64x.exe -DMODBUS_EXPORTS -q -O3 -DNDEBUG ...

With the enhancement, static library builds would show:

bcc64x.exe -q -O3 -DNDEBUG ...

Both produce identical static libraries, but the second more accurately represents what's being built.

Priority

This is a low-priority enhancement. The current implementation works correctly and there's no urgency to change it. It's merely a suggestion for potential future refinement if you're making other changes to the CMake configuration.

Additional Notes

I want to emphasize that this is completely optional feedback. The current design demonstrates good defensive programming with the layered checks in ModbusGlobal.h. Whether or not this enhancement is implemented, ModbusLib remains a solid, well-functioning library.

Thank you for your excellent work on this project, and please feel free to close this issue if you prefer the current approach!

Version Tested

• ModbusLib: 0.4.6
• Build System: CMake 4.1.1
• Compilers Tested: Embarcadero bcc64x (Clang 20.1.7), GCC, Clang

conditional-exports-define.patch