On 19/05/11 01:12, Harald Welte wrote: > Hi all, > > On Wed, May 18, 2011 at 04:10:20PM +0200, Holger Freyther wrote: > >> The easiest guess is that we either need the va_start/va_end inside the loop >> of _output, or need to copy the arguments again. to reproduce we should >> probably use the same arch and GCC/(E)GLIBC version that harald is using. > > My testing is on Debian unstable on x86_64 (libc6-2.13-4). As > indicated, reverting the commit Pablo indicated has fixed the problem. > > I have not seen the problem manifset itself on Ubuntu 11.04 on i386.
While reading the manpages, I noticed this: "The functions vprintf(), vfprintf(), vsprintf(), vsnprintf() are equivalent to the functions printf(), fprintf(), sprintf(), snprintf(), respectively, except that they are called with a va_list instead of a variable number of arguments. These functions do not call the va_end macro. Consequently, the value of ap is undefined after the call. The application should call va_end(ap) itself afterwards." I have attached a patch, it basically restores: http://cgit.osmocom.org/cgit/libosmocore/commit/?id=81e9636454294ae10ef9bc8bf149dd0248afce76 But it includes a better notice. Please, apply. Thanks!
logging: fix corrupted output From: Pablo Neira Ayuso <[email protected]> Harald reported a problem in the logging: http://lists.osmocom.org/pipermail/openbsc/2011-May/002896.html Reverting 81e9636454294ae10ef9bc8bf149dd0248afce76 seems to fix the problem. However, that workaround looks ugly. Holger gives us another clue on what was wrong: http://lists.osmocom.org/pipermail/openbsc/2011-May/002905.html While digging in the manpage, I found this: "The functions vprintf(), vfprintf(), vsprintf(), vsnprintf() are equivalent to the functions printf(), fprintf(), sprintf(), snprintf(), respectively, except that they are called with a va_list instead of a variable number of arguments. These functions do not call the va_end macro. Consequently, the value of ap is undefined after the call. The application should call va_end(ap) itself afterwards." --- src/logging.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/logging.c b/src/logging.c index 0911010..3c9dc03 100644 --- a/src/logging.c +++ b/src/logging.c @@ -198,6 +198,7 @@ static void _logp(unsigned int subsys, int level, char *file, int line, llist_for_each_entry(tar, &osmo_log_target_list, entry) { struct log_category *category; int output = 0; + va_list bp; category = &tar->categories[subsys]; /* subsystem is not supposed to be logged */ @@ -224,7 +225,12 @@ static void _logp(unsigned int subsys, int level, char *file, int line, if (!output) continue; + /* According to the manpage, vsnprintf leaves the value of ap + * in undefined state. Since _output uses vsnprintf and it may + * be called several times, we have to pass a copy of ap. */ + va_copy(bp, ap); _output(tar, subsys, level, file, line, cont, format, ap); + va_end(bp); } }
