Fix linking errors with ULOG_BUILD_DISABLED=1

[sobc]

Hey,

When using the ulog library with a C-file and -DULOG_BUILD_DISABLED=1 flag set, there are linking errors because the function definition is included for each translation unit.

Here is a MWE:

cmake_minimum_required(VERSION 3.10)
project(HelloWorld)

add_subdirectory(microlog EXCLUDE_FROM_ALL)
target_compile_definitions(microlog INTERFACE ULOG_BUILD_DISABLED=1)

add_executable(hello_world main.c)
target_link_libraries(hello_world PRIVATE microlog)

With Microlog included as a git clone beforehand, the main file consists of:

#include "ulog.h"

int main() {
  ulog_info("Hello World!l");
  return 0;
}

This runs fine in C++ because multiple identical inline declarations are allowed. However, during C linkage, it is not. To solve this issue, each function must be declared as static (inline) so that no ulog-related functions are exported during translation.

Please let me know if you run into any problems. :)

[an-dr]

Hey! Nice catch!!! Thanks. Try my proposal, don't think we need to inline all other declarations 😊

[an-dr]

I bet if you do only ULOG_INLINE -> ULOG_STATIC_INLINE it should solve the problem

#if ULOG_BUILD_DISABLED == 0
#define ULOG_STATIC_INLINE
#elif defined(__GNUC__) || defined(__clang__) || defined(__TI_COMPILER_VERSION__)
#define ULOG_STATIC_INLINE static inline __attribute__((always_inline))
#elif defined(_MSC_VER)
#define ULOG_STATIC_INLINE static __forceinline
#elif defined(__IAR_SYSTEMS_ICC__)
#define ULOG_STATIC_INLINE static inline _Pragma("inline=forced")
#else
#define ULOG_STATIC_INLINE static inline
#endif

[sobc]

I agree with you and wish this would work. So, are you suggesting that we simply add static to the inline keyword and remove static inline from the function declaration? ... And of course add STATIC the pre-processor macro 🙂

I gave it a try, but now the declaration is non-static while the definition is static. This leads to multiple definitions of the same symbol. For example:

In file included from /home/max/ulogtest/main.cpp:1:
/home/max/ulogtest/microlog/include/ulog.h:449:25: error: ‘ulog_status ulog_cleanup()’ was declared ‘extern’ and later ‘static’ [-fpermissive]
  449 | ULOG_INLINE ulog_status ulog_cleanup(void)

Thus, instead of static inline the declarations, just add the static keyword if ULOG_BUILD_DISABLED is defined.

What do you think about this?

[an-dr]

I dont think this is a redeclaration. It it about functions defined with and without static. Try to add guards around functions (only). Like this, by sections:

/* ============================================================================
   Core: Level
============================================================================ */

/// @brief Log levels in ascending order of severity
typedef enum ulog_level_enum {
    ULOG_LEVEL_0,  ///< Default ULOG_LEVEL_TRACE level
    ULOG_LEVEL_1,  ///< Default ULOG_LEVEL_DEBUG level
    ULOG_LEVEL_2,  ///< Default ULOG_LEVEL_INFO level
    ULOG_LEVEL_3,  ///< Default ULOG_LEVEL_WARN level
    ULOG_LEVEL_4,  ///< Default ULOG_LEVEL_ERROR level
    ULOG_LEVEL_5,  ///< Default ULOG_LEVEL_FATAL level
    ULOG_LEVEL_6,  ///< Custom level 6
    ULOG_LEVEL_7,  ///< Custom level 7
    ULOG_LEVEL_TOTAL,
} ulog_level;

/// @brief Default log levels in ascending order of severity
#define ULOG_LEVEL_TRACE (ulog_level)0  ///< For tracing execution
#define ULOG_LEVEL_DEBUG (ulog_level)1  ///< Debug information for developers
#define ULOG_LEVEL_INFO (ulog_level)2   ///< General information messages
#define ULOG_LEVEL_WARN (ulog_level)3   ///< For potential issues
#define ULOG_LEVEL_ERROR (ulog_level)4  ///< For failures
#define ULOG_LEVEL_FATAL (ulog_level)5  ///< May terminate program

typedef const char *ulog_level_names[ULOG_LEVEL_TOTAL];

typedef struct {
    ulog_level max_level;    // maximum log level
    ulog_level_names names;  // level names
} ulog_level_descriptor;

#if ULOG_BUILD_DISABLED != 1  //📍 that guard
/// @brief Returns the string representation of the log level
/// @param level Log level to convert
/// @return String representation of the level, or "?" for invalid levels
const char *ulog_level_to_string(ulog_level level);

/// @brief  Set custom log levels
/// @param new_levels
/// @return ulog_status
ulog_status ulog_level_set_new_levels(const ulog_level_descriptor *new_levels);

/// @brief Set default log levels (TRACE, DEBUG, INFO, WARN, ERROR, FATAL)
/// @return ulog_status
ulog_status ulog_level_reset_levels(void);
#endif  // ULOG_BUILD_DISABLED != 1 📍 that guard

[sobc]

Oh, I didn't see that!
... Thank you for your feedback. I will create another version based on your suggestion :)

[an-dr]

ALso dont worry about the integration test failing from your fork. It is a tech debt issue

[an-dr]

Merged! Thank you for your fix! ❤️