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>

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

diff --git a/tests/.gitignore b/tests/.gitignore
index 31fb70a484d364baf98d57fa45b7ddc6e9f2b456..8d00a3df7c8bbc3768d8ff380447078def5e77e7 100644 (file)
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -29,6 +29,7 @@ testbuffer
 testchecksum
 testmemory
 testprivs
+testsegv
 testsig
 teststream
 testnexthopiter

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

diff --git a/tests/test-segv.c b/tests/test-segv.c
new file mode 100644 (file)
index 0000000..55bd25a
--- /dev/null
+++ b/tests/test-segv.c
@@ -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);
+}