[Git][NTPsec/ntpsec][master] 9 commits: assert: remove indirection by *isc_assertion_failed.

Gary E. Miller gitlab at mg.gitlab.com
Fri Jun 2 21:53:37 UTC 2017


Gary E. Miller pushed to branch master at NTPsec / ntpsec


Commits:
aaa57b33 by Gary E. Miller at 2017-06-02T14:03:48-07:00
assert: remove indirection by *isc_assertion_failed.

ntpd only ever installed one assertion handler.  Before that pointer was
installed at runtime the assertions did not work well.  So remove all the
added indirection and just use assertion_failed().

Had to move assertion_failed() to libisc/assert.c to get it to work.

- - - - -
eb46eb45 by Gary E. Miller at 2017-06-02T14:24:32-07:00
assert: remove now unused isc_assertioncallback_t stuff.

Now that the assertion handler is hard coded, the stuff to
change the assertion handler can go away.  Which shows up
yet more unused code.

- - - - -
a040a1fd by Gary E. Miller at 2017-06-02T14:29:52-07:00
assert: remove default_callback().

It was only used until assertion_failed() was installed early in
startup.  Probably never worked, did not use mysyslog().

- - - - -
64ea95e0 by Gary E. Miller at 2017-06-02T14:32:48-07:00
assert: remove unused BACKTRACE_MAXFRAME

It was never used since the backtrace handlers were not installed.

- - - - -
b7a9dd68 by Gary E. Miller at 2017-06-02T14:35:30-07:00
assert: remove now unused prototype of isc_assertion_failed()

- - - - -
24dbf418 by Gary E. Miller at 2017-06-02T14:37:29-07:00
assert: make isc_assertion_typetotext() static.

- - - - -
3e314c9a by Gary E. Miller at 2017-06-02T14:39:47-07:00
assert: remove isc_ prefixes.

Not really ISC code anymore, does not look like their current code,
will keep their copyright.

- - - - -
d6034c4a by Gary E. Miller at 2017-06-02T14:43:18-07:00
assert: move libisc/assertions.c to libntp/assert.c

Not really ISC code anymore.

- - - - -
cb80377c by Gary E. Miller at 2017-06-02T14:48:38-07:00
assert: replace DEBUG_INSIST() with INSIST().

It was only used one place.  Also change DEBUG_ENSURE()
and DEBUG_REQUIRE(), which were all commented out anyway.

- - - - -


8 changed files:

- include/ntp_assert.h
- libisc/wscript
- libisc/assertions.c → libntp/assert.c
- libntp/authkeys.c
- libntp/timetoa.c
- libntp/wscript
- ntpd/ntp_monitor.c
- ntpd/ntpd.c


Changes:

=====================================
include/ntp_assert.h
=====================================
--- a/include/ntp_assert.h
+++ b/include/ntp_assert.h
@@ -24,9 +24,6 @@
  *
  * open question: when would we use INVARIANT()?
  *
- * For cases where the overhead for non-debug builds is deemed too high,
- * use DEBUG_REQUIRE(), DEBUG_INSIST(), DEBUG_ENSURE(), and/or
- * DEBUG_INVARIANT().
  */
 
 #ifndef GUARD_NTP_ASSERT_H
@@ -47,58 +44,37 @@
 
 /*% isc assertion type */
 typedef enum {
-	isc_assertiontype_require,
-	isc_assertiontype_ensure,
-	isc_assertiontype_insist,
-	isc_assertiontype_invariant
-} isc_assertiontype_t;
+	assertiontype_require,
+	assertiontype_ensure,
+	assertiontype_insist,
+	assertiontype_invariant
+} assertiontype_t;
 
-typedef void (*isc_assertioncallback_t)(const char *, int, isc_assertiontype_t,
-					const char *);
 
-/* coverity[+kill] */
-void isc_assertion_failed(const char *, int, isc_assertiontype_t,
-			  const char *) __attribute__((noreturn));
-
-void
-isc_assertion_setcallback(isc_assertioncallback_t);
-
-const char *
-isc_assertion_typetotext(isc_assertiontype_t type)
-			__attribute__((const));
+/* our assertion catcher */
+extern void	assertion_failed(const char *, int,
+				 assertiontype_t,
+				 const char *)
+			__attribute__	((__noreturn__));
 
 #define REQUIRE(cond) \
-	((void) ((cond) || \
-		 ((isc_assertion_failed)(__FILE__, __LINE__, \
-					 isc_assertiontype_require, \
+	((void) ((cond) || (assertion_failed(__FILE__, __LINE__, \
+					 assertiontype_require, \
 					 #cond), 0)))
 
 #define ENSURE(cond) \
-	((void) ((cond) || \
-		 ((isc_assertion_failed)(__FILE__, __LINE__, \
-					 isc_assertiontype_ensure, \
+	((void) ((cond) || (assertion_failed(__FILE__, __LINE__, \
+					 assertiontype_ensure, \
 					 #cond), 0)))
 
 #define INSIST(cond) \
-	((void) ((cond) || \
-		 ((isc_assertion_failed)(__FILE__, __LINE__, \
-					 isc_assertiontype_insist, \
+	((void) ((cond) || (assertion_failed(__FILE__, __LINE__, \
+					 assertiontype_insist, \
 					 #cond), 0)))
 #define INVARIANT(cond) \
-	((void) ((cond) || \
-		 ((isc_assertion_failed)(__FILE__, __LINE__, \
-					 isc_assertiontype_invariant, \
+	((void) ((cond) || (assertion_failed(__FILE__, __LINE__, \
+					 assertiontype_invariant, \
 					 #cond), 0)))
 # endif /* not FlexeLint */
 
-# ifdef DEBUG
-#define	DEBUG_REQUIRE(x)	REQUIRE(x)
-#define	DEBUG_INSIST(x)		INSIST(x)
-#define	DEBUG_ENSURE(x)		ENSURE(x)
-# else
-#define	DEBUG_REQUIRE(x)	do {} while (false)
-#define	DEBUG_INSIST(x)		do {} while (false)
-#define	DEBUG_ENSURE(x)		do {} while (false)
-# endif
-
 #endif	/* GUARD_NTP_ASSERT_H */


=====================================
libisc/wscript
=====================================
--- a/libisc/wscript
+++ b/libisc/wscript
@@ -2,7 +2,6 @@
 def build(ctx):
 
     libisc_source = [
-        "assertions.c",
         "backtrace.c",
         "error.c",
         "netaddr.c",


=====================================
libisc/assertions.c → libntp/assert.c
=====================================
--- a/libisc/assertions.c
+++ b/libntp/assert.c
@@ -12,52 +12,20 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "ntp.h"
+#include "ntp_debug.h"
+#include "ntp_syslog.h"
 #include "ntp_assert.h"
 #include "isc/backtrace.h"
 #include "isc/result.h"
 
-/*
- * The maximum number of stack frames to dump on assertion failure.
- */
-#ifndef BACKTRACE_MAXFRAME
-#define BACKTRACE_MAXFRAME 128
-#endif
-
-/*%
- * Forward.
- */
-static void
-default_callback(const char *, int, isc_assertiontype_t, const char *);
-
-static isc_assertioncallback_t isc_assertion_failed_cb = default_callback;
-
-/*%
- * Public.
- */
-
-/*% assertion failed handler */
-/* coverity[+kill] */
-void
-isc_assertion_failed(const char *file, int line, isc_assertiontype_t type,
-		     const char *cond)
-{
-	isc_assertion_failed_cb(file, line, type, cond);
-	abort();
-	/* NOTREACHED */
-}
-
-/*% Set callback. */
-void
-isc_assertion_setcallback(isc_assertioncallback_t cb) {
-	if (cb == NULL)
-		isc_assertion_failed_cb = default_callback;
-	else
-		isc_assertion_failed_cb = cb;
-}
+static const char *
+assertion_typetotext(assertiontype_t type)
+			__attribute__((const));
 
 /*% Type to Text */
-const char *
-isc_assertion_typetotext(isc_assertiontype_t type) {
+static const char *
+assertion_typetotext(assertiontype_t type) {
 	const char *result;
 
 	/*
@@ -66,16 +34,16 @@ isc_assertion_typetotext(isc_assertiontype_t type) {
 	 * the ISC development environment.
 	 */
 	switch (type) {
-	case isc_assertiontype_require:
+	case assertiontype_require:
 		result = "REQUIRE";
 		break;
-	case isc_assertiontype_ensure:
+	case assertiontype_ensure:
 		result = "ENSURE";
 		break;
-	case isc_assertiontype_insist:
+	case assertiontype_insist:
 		result = "INSIST";
 		break;
-	case isc_assertiontype_invariant:
+	case assertiontype_invariant:
 		result = "INVARIANT";
 		break;
 	default:
@@ -85,28 +53,23 @@ isc_assertion_typetotext(isc_assertiontype_t type) {
 }
 
 /*
- * Private.
+ * assertion_failed - Redirect assertion failed output to msyslog().
  */
-
-static void
-default_callback(const char *file, int line, isc_assertiontype_t type,
-		 const char *cond)
+void
+assertion_failed(
+	const char *file,
+	int line,
+	assertiontype_t type,
+	const char *cond
+	)
 {
-	void *tracebuf[BACKTRACE_MAXFRAME];
-	int i, nframes = 0;
-	const char *logsuffix = ".";
-	isc_result_t result;
+        /* Is recursion an issue? */
 
-	result = isc_backtrace_gettrace(tracebuf, BACKTRACE_MAXFRAME, &nframes);
-	if (result == ISC_R_SUCCESS && nframes > 0)
-		logsuffix = ", back trace";
+	termlogit = true; /* insist log to terminal */
 
-	fprintf(stderr, "%s:%d: %s(%s) failed%s\n",
-		file, line, isc_assertion_typetotext(type), cond, logsuffix);
-	if (result == ISC_R_SUCCESS) {
-		for (i = 0; i < nframes; i++) {
-			fprintf(stderr, "#%d %p in ??\n", i, tracebuf[i]);
-		}
-	}
-	fflush(stderr);
+	msyslog(LOG_ERR, "%s:%d: %s(%s) failed",
+		file, line, assertion_typetotext(type), cond);
+	msyslog(LOG_ERR, "exiting (due to assertion failure)");
+
+	abort();
 }


=====================================
libntp/authkeys.c
=====================================
--- a/libntp/authkeys.c
+++ b/libntp/authkeys.c
@@ -267,7 +267,7 @@ allocsymkey(
 	if (authnumfreekeys < 1)
 		auth_moremem(-1);
 	UNLINK_HEAD_SLIST(sk, authfreekeys, llink.f);
-	//DEBUG_ENSURE(sk != NULL);
+	//ENSURE(sk != NULL);
 	sk->keyid = id;
 	sk->flags = flags;
 	sk->type = type;
@@ -297,7 +297,7 @@ freesymkey(
                 sk->secret = NULL;
 	}
 	UNLINK_SLIST(unlinked, *bucket, sk, hlink, symkey);
-	//DEBUG_ENSURE(sk == unlinked);
+	//ENSURE(sk == unlinked);
 	UNLINK_DLIST(sk, llink);
 	memset((char *)sk + offsetof(symkey, symkey_payload), '\0',
 	       sizeof(*sk) - offsetof(symkey, symkey_payload));
@@ -457,8 +457,8 @@ mac_setkey(
 	uint8_t *	secret;
 	size_t		secretsize;
 	
-	//DEBUG_ENSURE(keytype <= USHRT_MAX);
-	//DEBUG_ENSURE(len < 4 * 1024);
+	//ENSURE(keytype <= USHRT_MAX);
+	//ENSURE(len < 4 * 1024);
 	/*
 	 * See if we already have the key.  If so just stick in the
 	 * new value.


=====================================
libntp/timetoa.c
=====================================
--- a/libntp/timetoa.c
+++ b/libntp/timetoa.c
@@ -62,7 +62,7 @@ format_time_fraction(
 	int		notneg;	/* flag for non-negative value	*/
 	ldiv_t		qr;
 
-	//DEBUG_REQUIRE(prec != 0);
+	//REQUIRE(prec != 0);
 
 	LIB_GETBUF(cp);
 	secs_u = (u_time)secs;
@@ -72,7 +72,7 @@ format_time_fraction(
 	prec_u = (u_int)abs(prec);
 	/* fraclimit = (long)pow(10, prec_u); */
 	for (fraclimit = 10, u = 1; u < prec_u; u++) {
-		//DEBUG_INSIST(fraclimit < fraclimit * 10);
+		//INSIST(fraclimit < fraclimit * 10);
 		fraclimit *= 10;
 	}
 


=====================================
libntp/wscript
=====================================
--- a/libntp/wscript
+++ b/libntp/wscript
@@ -2,6 +2,7 @@ def build(ctx):
     srcnode = ctx.srcnode.abspath()
 
     libntp_source = [
+        "assert.c",
         "authkeys.c",
         "authreadkeys.c",
         "clocktime.c",


=====================================
ntpd/ntp_monitor.c
=====================================
--- a/ntpd/ntp_monitor.c
+++ b/ntpd/ntp_monitor.c
@@ -157,7 +157,7 @@ mon_reclaim_entry(
 	mon_entry *m
 	)
 {
-	DEBUG_INSIST(NULL != m);
+	INSIST(NULL != m);
 
 	UNLINK_DLIST(m, mru);
 	remove_from_hash(m);


=====================================
ntpd/ntpd.c
=====================================
--- a/ntpd/ntpd.c
+++ b/ntpd/ntpd.c
@@ -112,10 +112,6 @@ static int	ntpdmain(int, char **) __attribute__((noreturn));
 static void	mainloop		(void)
 			__attribute__	((__noreturn__));
 static void	set_process_priority	(void);
-static void	assertion_failed	(const char *, int,
-					 isc_assertiontype_t,
-					 const char *)
-			__attribute__	((__noreturn__));
 static void	library_fatal_error	(const char *, int,
 					 const char *, va_list)
 					ISC_FORMAT_PRINTF(3, 0)
@@ -567,7 +563,6 @@ ntpdmain(
 	 * Install trap handlers to log errors and assertion failures.
 	 * Default handlers print to stderr which doesn't work if detached.
 	 */
-	isc_assertion_setcallback(assertion_failed);
 	isc_error_setfatal(library_fatal_error);
 	isc_error_setunexpected(library_unexpected_error);
 
@@ -1213,29 +1208,6 @@ static void check_minsane()
 }
 
 
-/*
- * assertion_failed - Redirect assertion failures to msyslog().
- */
-static void
-assertion_failed(
-	const char *file,
-	int line,
-	isc_assertiontype_t type,
-	const char *cond
-	)
-{
-        /* the comment on the next line is not what the line does */
-	isc_assertion_setcallback(NULL);    /* Avoid recursion */
-	/* isc_assertion_setcallback(NULL) just set the default assert()
-         * handler, allowing recursion */
-
-	msyslog(LOG_ERR, "%s:%d: %s(%s) failed",
-		file, line, isc_assertion_typetotext(type), cond);
-	msyslog(LOG_ERR, "exiting (due to assertion failure)");
-
-	abort();
-}
-
 
 /*
  * library_fatal_error - Handle fatal errors from our libraries.



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/dee3f727e0954f1aac9fe4b13225ef89364c2787...cb80377cf80dc32c335126a85ce5da342240274d

---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/dee3f727e0954f1aac9fe4b13225ef89364c2787...cb80377cf80dc32c335126a85ce5da342240274d
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20170602/95a7b4ea/attachment.html>


More information about the vc mailing list