OlegDB icon indicating copy to clipboard operation
OlegDB copied to clipboard

API Improvements

Open qpfiffer opened this issue 11 years ago • 6 comments

Hide all _ol functions when compiling library: http://gcc.gnu.org/wiki/Visibility We don't need to export functions like _ol_trunc that are used internally.

Also make the header CPP compatible.

qpfiffer avatar Feb 25 '14 07:02 qpfiffer

I couldn't find a way to hide functions in C nor C++.

For the header CPP compatibility, check #68

Hamcha avatar May 01 '14 00:05 Hamcha

@Hamcha Both sdb and sphia do something like the following:

#if __GNUC__ >= 4
#  define SP_API __attribute__((visibility("default")))
#else
#  define SP_API
#endif

...

SP_API void *random_function();

qpfiffer avatar May 01 '14 02:05 qpfiffer

I meant Standard C, not GNU style, I thought our coding conventions were against that? Stay POSIX compliant. No weird #define _GNU_SOURCE stuff. Or did that become a standard somehow?

UPDATE It looks like __attribute__((visibility("default"))) is supported on Clang as well, but won't work if #if __GNUC__ >= 4 is there.. maybe we should put it with some ifdefs to check for GCC 3 (if we really want to support that)?

Hamcha avatar May 01 '14 11:05 Hamcha

I think this is okay because A) It is support by both gcc and clang and B) It doesn't pull in any of the weird functions/implementations in _GNU_SOURCE. My hangups with _GNU_SOURCE are that it is both buggy and non-portable.

I see this as more of an optimization for people linking to binary libraries built with clang/gcc. Since it's wrapped in an #ifdef, it shouldn't break anything.

For now though, since liboleg isn't that big, we can probably skip it. This is as much a discussion issue as anything else.

qpfiffer avatar May 01 '14 15:05 qpfiffer

I think the problem here is the #ifdef itself, what are we going to test it against? __GNUC__ is not used by clang, but removing the check entirely will break GCC 3. Boost takes this whole thing a whole step further by using different headers for such things (gcc.hpp/clang.hpp/etc) but I think that would be overkill for this project.

Hamcha avatar May 01 '14 16:05 Hamcha

I think something like this would work:

#if defined (__GNUC__) || defined (__llvm__)
#  if __GNUC__ >= 4
#    define OL_API __attribute__((visibility("default")))
#  else
#    define OL_API
#  endif
#else
#  define OL_API
#endif

qpfiffer avatar May 01 '14 16:05 qpfiffer