falcosecurity / libs

libsinsp, libscap, the kernel module driver, and the eBPF driver sources
https://falcosecurity.github.io/libs/
Apache License 2.0
230 stars 164 forks source link

libsinsp/version.h is fragile with respect to use of __STDC_FORMAT_MACROS #346

Closed mstemm closed 2 years ago

mstemm commented 2 years ago

Describe the bug

version.h at commit c8af440082fd996ade25b2128167168db98710a5 is a bit fragile with respect to its use of __STDC_FORMAT_MACROS. If that preprocessor define is set before including intttypes.h, it allows adding formatting directives like PRIu32 to sprintf/printf/etc.

The problem is that STDC_FORMAT_MACROS must be defined before the first include of inttypes.h. If another header includes version.h but also includes inttypes.h before version.h without STDC_FORMAT_MACROS, the formatting directives won't be available.

I suppose you could fix all includes of inttypes.h in libs to ensure that __STDC_FORMAT_MACROS is defined, but I would think an easier solution would be to move the implementation of the methods into a version.c file and control the includes there.

How to reproduce it

I can't include the full build log, since this is a custom use of libs, but I do get this error:

XXX/userspace/libsinsp/version.h: In constructor 'sinsp_version::sinsp_version(const string&)':
XXX/userspace/libsinsp/version.h:39:45: error: expected ')' before 'PRIu32'
   m_valid = sscanf(version_str.c_str(), "%" PRIu32 ".%" PRIu32 ".%" PRIu32,

If I add __STDC_FORMAT_MACROS to the include of inttypes.h in userspace/libscap/plugin_info.h and userspace/libscap/uthash.h, I can build without this error.

Expected behaviour

Screenshots

Environment

Additional context

FedeDP commented 2 years ago

Hi Mark, thanks for discovering this!

I suppose you could fix all includes of inttypes.h in libs to ensure that __STDC_FORMAT_MACROS is defined, but I would think an easier solution would be to move the implementation of the methods into a version.c file and control the includes there.

Can't we define the symbol as a compiler flag instead? I mean, this way the symbol is globally visible and exported before anything else.