On 10/02/14 12:44, Rainer Gerhards wrote:
2014-10-02 12:01 GMT+02:00 Olaf <[email protected]>:
I've got a build error in 7.6.6:
<...>
I've hit this too... with the latest const *char variant.
There are actually multiple intertwined issues.
Summary and couple of suggestions are bellow.
offending line seems line 1462 in lexer.c:
extern char *strdup(char*); /* somehow we do not get this from
string.h... */
If I drop that line build is OK.
My string.h does have strdup, but defined as: extern char *strdup(__const
char*);
Yeah, I banged my head on this for many hours before I finally gave up. I
have some build environments where string.h contains what is needed, but
even if I explicitly include it, the prototype is not there! On 64 bit
systems, that can lead to a segfault (acutally, this was the problem we
sawn with 7.6.5 on CENTOS). Again, I never found an explanation, and I
would still like to get one...
The explanation is that strdup() is an extension to C99 and it is not
exported by default (when adhering to C99).
The actual issue is that config.h, which requests extensions to be used,
is included in lexer.c _after_ string.h is included.
In more detail:
The prototype of strdup() is given in string.h - but not
unconditionally. The man page of strdup() lists these prerequisites:
_SVID_SOURCE || _BSD_SOURCE || _XOPEN_SOURCE >= 500 || _XOPEN_SOURCE &&
_XOPEN_SOURCE_EXTENDED
Some of these macros are implied by _GNU_SOURCE, which is requested by
AC_GNU_SOURCE in configure.ac.
For this configuration to take effect, config.h must be included before
including string.h (and other headers).
'#include "config.h"' is pretty close to the top of lexer.l, but, for
some reason, flex prepends couple of standard headers (including
string.h) to the very top of the generated lexer.c.
I'm not very familiar with flex and it's conventions, but one possible
solutions would be to use "%top{ #include "config.h }".
Ad AC_GNU_SOURCE - the autoconf manual[0] claims it's obsolete and
suggests to use AC_USE_SYSTEM_EXTENSIONS instead. The new macro should
be available in the autoconf version already required by rsyslog.
Ad potential segfaults - how about adding this option to CFLAGS:
-Werror=implicit-function-declaration
Is there any case where implicit functions should be allowed?
There's a "regression" in the 8.3+ versions that affects the usage of
compiler extensions. If you just remove the strdup() prototype from
lexer.l, it will compile fine in my environment because gcc provides a
correct implicit prototype by default. The commit b195717 added an
explicit "-std=c99" flag which suppresses the default extensions and
results in an incorrect implicit prototype for strdup().
The explicit flag is not needed (anymore) - the commit 5d37791 added the
AC_PROG_CC_C99 macro to configure.ac which should be sufficient.
The explicit flag is actually harmful - if you look at the gcc
arguments, the -std flag is there twice. Once with "gnu99" (from
AC_PROG_CC_C99) and once with "c99". They appear in this order on the
command line and the later takes precedence. This unnecessarily disables
compiler features that would be available otherwise.
Following patch also works:
<...>
But I do not know if that matches all variants of strdup that may be out
there.
At least on the Fedora, openSuse, Ubuntu (thus Debian) I had quickly at
hand it worked. Which platform did you use?
I'd rather not include explicit prototypes of library functions in the
code as they may differ on various platforms. (On Fedora 20, compiling
v7.6.7 fails because strdup() is defined as a macro containing several
commands.)
I'll send a pull request implementing all the abovementioned changes if
there are no objections.
Thoughts?
Tomas
[0]
https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Posix-Variants.html
_______________________________________________
rsyslog mailing list
http://lists.adiscon.net/mailman/listinfo/rsyslog
http://www.rsyslog.com/professional-services/
What's up with rsyslog? Follow https://twitter.com/rgerhards
NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad of
sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you DON'T LIKE
THAT.
