From: Tony Finch <dot@dotat.at>
Date: Fri, 14 Oct 2022 16:37:47 +0000 (+0100)
Subject: Include the function name when reporting unexpected errors
X-Git-Tag: v9.19.7~56^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=26ed03a61e0cb6359a985784ffad93630cb2aeff;p=thirdparty%2Fbind9.git

Include the function name when reporting unexpected errors

I.e. print the name of the function in BIND that called the system
function that returned an error. Since it was useful for pthreads
code, it seems worthwhile doing so everywhere.
---

diff --git a/bin/named/main.c b/bin/named/main.c
index c928069d913..606c3ea858b 100644
--- a/bin/named/main.c
+++ b/bin/named/main.c
@@ -241,12 +241,12 @@ assertion_failed(const char *file, int line, isc_assertiontype_t type,
 }
 
 noreturn static void
-library_fatal_error(const char *file, int line, const char *format,
-		    va_list args) ISC_FORMAT_PRINTF(3, 0);
+library_fatal_error(const char *file, int line, const char *func,
+		    const char *format, va_list args) ISC_FORMAT_PRINTF(3, 0);
 
 static void
-library_fatal_error(const char *file, int line, const char *format,
-		    va_list args) {
+library_fatal_error(const char *file, int line, const char *func,
+		    const char *format, va_list args) {
 	/*
 	 * Handle isc_error_fatal() calls from our libraries.
 	 */
@@ -260,7 +260,7 @@ library_fatal_error(const char *file, int line, const char *format,
 
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_MAIN, ISC_LOG_CRITICAL,
-			      "%s:%d: fatal error:", file, line);
+			      "%s:%d:%s(): fatal error: ", file, line, func);
 		isc_log_vwrite(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			       NAMED_LOGMODULE_MAIN, ISC_LOG_CRITICAL, format,
 			       args);
@@ -268,7 +268,7 @@ library_fatal_error(const char *file, int line, const char *format,
 			      NAMED_LOGMODULE_MAIN, ISC_LOG_CRITICAL,
 			      "exiting (due to fatal error in library)");
 	} else {
-		fprintf(stderr, "%s:%d: fatal error: ", file, line);
+		fprintf(stderr, "%s:%d:%s(): fatal error: ", file, line, func);
 		vfprintf(stderr, format, args);
 		fprintf(stderr, "\n");
 		fflush(stderr);
@@ -281,12 +281,13 @@ library_fatal_error(const char *file, int line, const char *format,
 }
 
 static void
-library_unexpected_error(const char *file, int line, const char *format,
-			 va_list args) ISC_FORMAT_PRINTF(3, 0);
+library_unexpected_error(const char *file, int line, const char *func,
+			 const char *format, va_list args)
+	ISC_FORMAT_PRINTF(3, 0);
 
 static void
-library_unexpected_error(const char *file, int line, const char *format,
-			 va_list args) {
+library_unexpected_error(const char *file, int line, const char *func,
+			 const char *format, va_list args) {
 	/*
 	 * Handle isc_error_unexpected() calls from our libraries.
 	 */
@@ -294,12 +295,13 @@ library_unexpected_error(const char *file, int line, const char *format,
 	if (named_g_lctx != NULL) {
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_MAIN, ISC_LOG_ERROR,
-			      "%s:%d: unexpected error:", file, line);
+			      "%s:%d:%s(): unexpected error: ", file, line,
+			      func);
 		isc_log_vwrite(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			       NAMED_LOGMODULE_MAIN, ISC_LOG_ERROR, format,
 			       args);
 	} else {
-		fprintf(stderr, "%s:%d: fatal error: ", file, line);
+		fprintf(stderr, "%s:%d:%s(): fatal error: ", file, line, func);
 		vfprintf(stderr, format, args);
 		fprintf(stderr, "\n");
 		fflush(stderr);
diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c
index 7c3b57abb5c..7641751f790 100644
--- a/bin/tests/system/dyndb/driver/db.c
+++ b/bin/tests/system/dyndb/driver/db.c
@@ -133,7 +133,7 @@ beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) {
 	UNUSED(db);
 	UNUSED(callbacks);
 
-	fatal_error("current implementation should never call beginload()");
+	FATAL_ERROR("current implementation should never call beginload()");
 
 	/* Not reached */
 	return (ISC_R_SUCCESS);
@@ -149,7 +149,7 @@ endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) {
 	UNUSED(db);
 	UNUSED(callbacks);
 
-	fatal_error("current implementation should never call endload()");
+	FATAL_ERROR("current implementation should never call endload()");
 
 	/* Not reached */
 	return (ISC_R_SUCCESS);
@@ -163,7 +163,7 @@ dump(dns_db_t *db, dns_dbversion_t *version, const char *filename,
 	UNUSED(filename);
 	UNUSED(masterformat);
 
-	fatal_error("current implementation should never call dump()");
+	FATAL_ERROR("current implementation should never call dump()");
 
 	/* Not reached */
 	return (ISC_R_SUCCESS);
diff --git a/bin/tests/system/dyndb/driver/log.h b/bin/tests/system/dyndb/driver/log.h
index 2cb968bf81a..375db2b46ad 100644
--- a/bin/tests/system/dyndb/driver/log.h
+++ b/bin/tests/system/dyndb/driver/log.h
@@ -34,8 +34,6 @@
 
 #include <dns/log.h>
 
-#define fatal_error(...) isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
-
 #define log_error_r(fmt, ...) \
 	log_error(fmt ": %s", ##__VA_ARGS__, isc_result_totext(result))
 
diff --git a/bin/tests/system/dyndb/driver/syncptr.c b/bin/tests/system/dyndb/driver/syncptr.c
index 81d98e5d43c..5124df32b4d 100644
--- a/bin/tests/system/dyndb/driver/syncptr.c
+++ b/bin/tests/system/dyndb/driver/syncptr.c
@@ -157,7 +157,7 @@ syncptr_find_zone(sample_instance_t *inst, dns_rdata_t *rdata, dns_name_t *name,
 		break;
 
 	default:
-		fatal_error("unsupported address type 0x%x", rdata->type);
+		FATAL_ERROR("unsupported address type 0x%x", rdata->type);
 		break;
 	}
 
diff --git a/lib/isc/error.c b/lib/isc/error.c
index 336be8432ee..051a6c48aae 100644
--- a/lib/isc/error.c
+++ b/lib/isc/error.c
@@ -21,12 +21,12 @@
 
 /*% Default unexpected callback. */
 static void
-default_unexpected_callback(const char *, int, const char *, va_list)
-	ISC_FORMAT_PRINTF(3, 0);
+default_unexpected_callback(const char *, int, const char *, const char *,
+			    va_list) ISC_FORMAT_PRINTF(4, 0);
 
 /*% Default fatal callback. */
 static void
-default_fatal_callback(const char *, int, const char *, va_list)
+default_fatal_callback(const char *, int, const char *, const char *, va_list)
 	ISC_FORMAT_PRINTF(3, 0);
 
 /*% unexpected_callback */
@@ -52,42 +52,39 @@ isc_error_setfatal(isc_errorcallback_t cb) {
 }
 
 void
-isc_error_unexpected(const char *file, int line, const char *format, ...) {
+isc_error_unexpected(const char *file, int line, const char *func,
+		     const char *format, ...) {
 	va_list args;
 
 	va_start(args, format);
-	(unexpected_callback)(file, line, format, args);
+	(unexpected_callback)(file, line, func, format, args);
 	va_end(args);
 }
 
 void
-isc_error_fatal(const char *file, int line, const char *format, ...) {
+isc_error_fatal(const char *file, int line, const char *func,
+		const char *format, ...) {
 	va_list args;
 
 	va_start(args, format);
-	(fatal_callback)(file, line, format, args);
+	(fatal_callback)(file, line, func, format, args);
 	va_end(args);
 	abort();
 }
 
-void
-isc_error_runtimecheck(const char *file, int line, const char *expression) {
-	isc_error_fatal(file, line, "RUNTIME_CHECK(%s) failed", expression);
-}
-
 static void
-default_unexpected_callback(const char *file, int line, const char *format,
-			    va_list args) {
-	fprintf(stderr, "%s:%d: ", file, line);
+default_unexpected_callback(const char *file, int line, const char *func,
+			    const char *format, va_list args) {
+	fprintf(stderr, "%s:%d:%s(): ", file, line, func);
 	vfprintf(stderr, format, args);
 	fprintf(stderr, "\n");
 	fflush(stderr);
 }
 
 static void
-default_fatal_callback(const char *file, int line, const char *format,
-		       va_list args) {
-	fprintf(stderr, "%s:%d: fatal error: ", file, line);
+default_fatal_callback(const char *file, int line, const char *func,
+		       const char *format, va_list args) {
+	fprintf(stderr, "%s:%d:%s(): fatal error: ", file, line, func);
 	vfprintf(stderr, format, args);
 	fprintf(stderr, "\n");
 	fflush(stderr);
diff --git a/lib/isc/include/isc/error.h b/lib/isc/include/isc/error.h
index 96552bcb78d..f4f668203f3 100644
--- a/lib/isc/include/isc/error.h
+++ b/lib/isc/include/isc/error.h
@@ -23,7 +23,8 @@
 
 ISC_LANG_BEGINDECLS
 
-typedef void (*isc_errorcallback_t)(const char *, int, const char *, va_list);
+typedef void (*isc_errorcallback_t)(const char *, int, const char *,
+				    const char *, va_list);
 
 /*% set unexpected error */
 void isc_error_setunexpected(isc_errorcallback_t);
@@ -33,15 +34,12 @@ void isc_error_setfatal(isc_errorcallback_t);
 
 /*% unexpected error */
 void
-isc_error_unexpected(const char *, int, const char *, ...)
-	ISC_FORMAT_PRINTF(3, 4);
+isc_error_unexpected(const char *, int, const char *, const char *, ...)
+	ISC_FORMAT_PRINTF(4, 5);
 
 /*% fatal error */
 noreturn void
-isc_error_fatal(const char *, int, const char *, ...) ISC_FORMAT_PRINTF(3, 4);
-
-/*% runtimecheck error */
-noreturn void
-isc_error_runtimecheck(const char *, int, const char *);
+isc_error_fatal(const char *, int, const char *, const char *, ...)
+	ISC_FORMAT_PRINTF(4, 5);
 
 ISC_LANG_ENDDECLS
diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h
index 38b025b392e..f24a8da2d58 100644
--- a/lib/isc/include/isc/util.h
+++ b/lib/isc/include/isc/util.h
@@ -316,16 +316,17 @@ mock_assert(const int result, const char *const expression,
 #include <isc/strerr.h> /* for ISC_STRERRORSIZE */
 
 #define UNEXPECTED_ERROR(...) \
-	isc_error_unexpected(__FILE__, __LINE__, __VA_ARGS__)
+	isc_error_unexpected(__FILE__, __LINE__, __func__, __VA_ARGS__)
 
-#define FATAL_ERROR(...) isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
+#define FATAL_ERROR(...) \
+	isc_error_fatal(__FILE__, __LINE__, __func__, __VA_ARGS__)
 
-#define REPORT_SYSERROR(report, err, fmt, ...)                             \
-	{                                                                  \
-		char _strerr[ISC_STRERRORSIZE];                            \
-		strerror_r(err, _strerr, sizeof(_strerr));                 \
-		report(__FILE__, __LINE__, fmt ": %s (%d)", ##__VA_ARGS__, \
-		       _strerr, err);                                      \
+#define REPORT_SYSERROR(report, err, fmt, ...)                        \
+	{                                                             \
+		char strerr[ISC_STRERRORSIZE];                        \
+		strerror_r(err, strerr, sizeof(strerr));              \
+		report(__FILE__, __LINE__, __func__, fmt ": %s (%d)", \
+		       ##__VA_ARGS__, strerr, err);                   \
 	}
 
 #define UNEXPECTED_SYSERROR(err, ...) \
@@ -343,7 +344,7 @@ mock_assert(const int result, const char *const expression,
 #else /* UNIT_TESTING */
 
 #define RUNTIME_CHECK(cond) \
-	((cond) ? (void)0 : isc_error_runtimecheck(__FILE__, __LINE__, #cond))
+	((cond) ? (void)0 : FATAL_ERROR("RUNTIME_CHECK(%s) failed", #cond))
 
 #endif /* UNIT_TESTING */
 
@@ -351,9 +352,9 @@ mock_assert(const int result, const char *const expression,
  * Runtime check which logs the error value returned by a POSIX Threads
  * function and the error string that corresponds to it
  */
-#define PTHREADS_RUNTIME_CHECK(func, ret)                           \
-	if ((ret) != 0) {                                           \
-		FATAL_SYSERROR(ret, "%s(): %s()", __func__, #func); \
+#define PTHREADS_RUNTIME_CHECK(func, ret)           \
+	if ((ret) != 0) {                           \
+		FATAL_SYSERROR(ret, "%s()", #func); \
 	}
 
 /*%
diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index 17040c7c9be..043a60e2464 100644
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -1249,9 +1249,7 @@ isc__mempool_destroy(isc_mempool_t **restrict mpctxp FLARG) {
 #endif
 
 	if (mpctx->allocated > 0) {
-		UNEXPECTED_ERROR(
-			"isc_mempool_destroy(): mempool %s leaked memory",
-			mpctx->name);
+		UNEXPECTED_ERROR("mempool %s leaked memory", mpctx->name);
 	}
 	REQUIRE(mpctx->allocated == 0);