lib: fix backtraces broken by 837d16c...

837d16c ("*: use array_size() helper macro") accidentally changed one of
the expressions in the backtrace code, which afterwards read:

zlog_backtrace_sigsafe():
  if (((size = backtrace(array,array_size(array)) <= 0) ||

which boils down to: (size = backtrace(...)  <= 0).  The braces were
intended to go:      (size = backtrace(...)) <= 0.

All in all, this makes a nice textbook example of the original author
being too clever (trying to save a single line by pulling the assignment
into the condition) and the next person touching the code tripping over
it...

This code occurs another time in zlog_backtrace() where it is actually
correct.  Pulling out the assignment nonetheless.  Also, new test
program.

Cc: Andrew J. Schorr <ajschorr@alumni.princeton.edu>
Cc: Balaji.G <balajig81@gmail.com>
Cc: Scott Feldman <sfeldma@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This commit is contained in:
David Lamparter 2013-11-19 15:00:06 +01:00
parent c78a46c27f
commit 4d474fa329
4 changed files with 69 additions and 5 deletions

View File

@ -443,8 +443,8 @@ zlog_backtrace_sigsafe(int priority, void *program_counter)
#define LOC s,buf+sizeof(buf)-s #define LOC s,buf+sizeof(buf)-s
#ifdef HAVE_GLIBC_BACKTRACE #ifdef HAVE_GLIBC_BACKTRACE
if (((size = backtrace(array,array_size(array)) <= 0) || size = backtrace(array, array_size(array));
((size_t)size > array_size(array)))) if (size <= 0 || (size_t)size > array_size(array))
return; return;
#define DUMP(FD) { \ #define DUMP(FD) { \
@ -526,8 +526,8 @@ zlog_backtrace(int priority)
int size, i; int size, i;
char **strings; char **strings;
if (((size = backtrace(array,array_size(array))) <= 0) || size = backtrace(array, array_size(array));
((size_t)size > array_size(array))) if (size <= 0 || (size_t)size > array_size(array))
{ {
zlog_err("Cannot get backtrace, returned invalid # of frames %d " zlog_err("Cannot get backtrace, returned invalid # of frames %d "
"(valid range is between 1 and %lu)", "(valid range is between 1 and %lu)",

1
tests/.gitignore vendored
View File

@ -29,6 +29,7 @@ testbuffer
testchecksum testchecksum
testmemory testmemory
testprivs testprivs
testsegv
testsig testsig
teststream teststream
testnexthopiter testnexthopiter

View File

@ -24,13 +24,14 @@ else
TESTS_BGPD = TESTS_BGPD =
endif endif
check_PROGRAMS = testsig testbuffer testmemory heavy heavywq heavythread \ check_PROGRAMS = testsig testsegv testbuffer testmemory heavy heavywq heavythread \
testprivs teststream testchecksum tabletest testnexthopiter \ testprivs teststream testchecksum tabletest testnexthopiter \
$(TESTS_BGPD) $(TESTS_BGPD)
noinst_HEADERS = prng.h noinst_HEADERS = prng.h
testsig_SOURCES = test-sig.c testsig_SOURCES = test-sig.c
testsegv_SOURCES = test-segv.c
testbuffer_SOURCES = test-buffer.c testbuffer_SOURCES = test-buffer.c
testmemory_SOURCES = test-memory.c testmemory_SOURCES = test-memory.c
testprivs_SOURCES = test-privs.c testprivs_SOURCES = test-privs.c
@ -48,6 +49,7 @@ tabletest_SOURCES = table_test.c
testnexthopiter_SOURCES = test-nexthop-iter.c prng.c testnexthopiter_SOURCES = test-nexthop-iter.c prng.c
testsig_LDADD = ../lib/libzebra.la @LIBCAP@ testsig_LDADD = ../lib/libzebra.la @LIBCAP@
testsegv_LDADD = ../lib/libzebra.la @LIBCAP@
testbuffer_LDADD = ../lib/libzebra.la @LIBCAP@ testbuffer_LDADD = ../lib/libzebra.la @LIBCAP@
testmemory_LDADD = ../lib/libzebra.la @LIBCAP@ testmemory_LDADD = ../lib/libzebra.la @LIBCAP@
testprivs_LDADD = ../lib/libzebra.la @LIBCAP@ testprivs_LDADD = ../lib/libzebra.la @LIBCAP@

61
tests/test-segv.c Normal file
View File

@ -0,0 +1,61 @@
/*
* SEGV / backtrace handling test.
*
* copied from test-sig.c
*
* Copyright (C) 2013 by David Lamparter, Open Source Routing.
* Copyright (C) 2013 by Internet Systems Consortium, Inc. ("ISC")
*
* This file is part of Quagga
*
* Quagga is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2, or (at your option) any
* later version.
*
* Quagga is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Quagga; see the file COPYING. If not, write to the Free
* Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
* 02111-1307, USA.
*/
#include <zebra.h>
#include <sigevent.h>
#include "lib/log.h"
#include "lib/memory.h"
struct quagga_signal_t sigs[] =
{
};
struct thread_master *master;
int
threadfunc (struct thread *thread)
{
int *null = NULL;
*null += 1;
return 0;
}
int
main (void)
{
master = thread_master_create ();
signal_init (master, array_size(sigs), sigs);
zlog_default = openzlog("testsegv", ZLOG_NONE,
LOG_CONS|LOG_NDELAY|LOG_PID, LOG_DAEMON);
zlog_set_level (NULL, ZLOG_DEST_SYSLOG, ZLOG_DISABLED);
zlog_set_level (NULL, ZLOG_DEST_STDOUT, LOG_DEBUG);
zlog_set_level (NULL, ZLOG_DEST_MONITOR, ZLOG_DISABLED);
thread_execute (master, threadfunc, 0, 0);
exit (0);
}