rsyslog / liblognorm

a fast samples-based log normalization library
http://www.liblognorm.com
GNU Lesser General Public License v2.1
99 stars 64 forks source link

strndup definition must not be part of the public API #214

Closed mbiebl closed 8 years ago

mbiebl commented 8 years ago

liblognorm.h contains the following:

#ifndef HAVE_STRNDUP
char * strndup(const char *s, size_t n);
#endif

This is wrong, though. You can't assume that consumers of your API actually check for the existence of strndup in their configure script and set/unset HAVE_STRNDUP.

The result is, that HAVE_STRNDUP will be unset. This e.g. leads to

make[2]: Entering directory '/tmp/sagan-1.0.1/src'
gcc -DHAVE_CONFIG_H -I. -I..  -I..  -I/usr/include/libfastjson    -g
-O2 -MT sagan-sagan-config.o -MD -MP -MF .deps/sagan-sagan-config.Tpo
-c -o sagan-sagan-config.o `test -f 'sagan-config.c' || echo
'./'`sagan-config.c
In file included from /usr/include/string.h:630:0,
                 from sagan-config.c:40:
/usr/include/liblognorm.h:262:8: error: expected identifier or '('
before '__extension__'
 char * strndup(const char *s, size_t n);
        ^
Makefile:588: recipe for target 'sagan-sagan-config.o' failed
make[2]: *** [sagan-sagan-config.o] Error 1
make[2]: Leaving directory '/tmp/sagan-1.0.1/src'
Makefile:390: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/tmp/sagan-1.0.1'
Makefile:331: recipe for target 'all' failed
make: *** [all] Error 2

You should define the compat strndup in a file like compat.h, which is not included in the public API. C files which use strndup should include that compat.h header.

mbiebl commented 8 years ago

The commit which introduced this is f2a8d45d9d4f9bfdba3bb1775b99fdb9fbb1e903

mbiebl commented 8 years ago
/* here we add some stuff from the compatibility layer. A separate include
 * would be cleaner, but would potentially require changes all over the
 * place. So doing it here is better.

Adding it to the public API is not better but plain wrong. There might be a possibility to inject that via config.h, if you are concerned about having to include a separate header file.