From ec50c58f5205abf076152d51f9192c807920add5 Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Fri, 14 Oct 2022 16:07:07 +0100 Subject: [PATCH 1/4] De-duplicate __FILE__, __LINE__ Mostly generated automatically with the following semantic patch, except where coccinelle was confused by #ifdef in lib/isc/net.c @@ expression list args; @@ - UNEXPECTED_ERROR(__FILE__, __LINE__, args) + UNEXPECTED_ERROR(args) @@ expression list args; @@ - FATAL_ERROR(__FILE__, __LINE__, args) + FATAL_ERROR(args) --- bin/named/dlz_dlopen_driver.c | 3 +-- bin/named/main.c | 18 ++++++------------ bin/named/server.c | 3 +-- doc/dev/unexpected | 3 +-- lib/dns/cache.c | 18 ++++++------------ lib/dns/diff.c | 3 +-- lib/dns/journal.c | 2 +- lib/dns/master.c | 27 +++++++++------------------ lib/dns/masterdump.c | 21 +++++++-------------- lib/dns/name.c | 12 ++++-------- lib/dns/rbtdb.c | 2 +- lib/dns/resolver.c | 9 +++------ lib/dns/rpz.c | 2 +- lib/dns/view.c | 6 ++---- lib/dns/zone.c | 3 +-- lib/isc/condition.c | 3 +-- lib/isc/include/isc/util.h | 9 ++++----- lib/isc/interfaceiter.c | 3 +-- lib/isc/lex.c | 3 +-- lib/isc/mem.c | 7 +++---- lib/isc/net.c | 29 +++++++++++------------------ lib/isc/rwlock.c | 3 +-- lib/isc/sockaddr.c | 9 ++++----- lib/isc/time.c | 4 ++-- lib/isc/tls.c | 3 +-- lib/ns/interfacemgr.c | 3 +-- lib/ns/sortlist.c | 7 +++---- lib/ns/update.c | 7 +++---- 28 files changed, 81 insertions(+), 141 deletions(-) diff --git a/bin/named/dlz_dlopen_driver.c b/bin/named/dlz_dlopen_driver.c index 794deb38a1..0d5bcced64 100644 --- a/bin/named/dlz_dlopen_driver.c +++ b/bin/named/dlz_dlopen_driver.c @@ -531,8 +531,7 @@ dlz_dlopen_init(isc_mem_t *mctx) { mctx, &dlz_dlopen); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "dns_sdlzregister() failed: %s", + UNEXPECTED_ERROR("dns_sdlzregister() failed: %s", isc_result_totext(result)); result = ISC_R_UNEXPECTED; } diff --git a/bin/named/main.c b/bin/named/main.c index 0f3896e00b..c928069d91 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -1406,8 +1406,7 @@ named_smf_get_instance(char **ins_name, int debug, isc_mem_t *mctx) { if ((h = scf_handle_create(SCF_VERSION)) == NULL) { if (debug) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "scf_handle_create() failed: %s", + UNEXPECTED_ERROR("scf_handle_create() failed: %s", scf_strerror(scf_error())); } return (ISC_R_FAILURE); @@ -1415,8 +1414,7 @@ named_smf_get_instance(char **ins_name, int debug, isc_mem_t *mctx) { if (scf_handle_bind(h) == -1) { if (debug) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "scf_handle_bind() failed: %s", + UNEXPECTED_ERROR("scf_handle_bind() failed: %s", scf_strerror(scf_error())); } scf_handle_destroy(h); @@ -1425,8 +1423,7 @@ named_smf_get_instance(char **ins_name, int debug, isc_mem_t *mctx) { if ((namelen = scf_myname(h, NULL, 0)) == -1) { if (debug) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "scf_myname() failed: %s", + UNEXPECTED_ERROR("scf_myname() failed: %s", scf_strerror(scf_error())); } scf_handle_destroy(h); @@ -1434,8 +1431,7 @@ named_smf_get_instance(char **ins_name, int debug, isc_mem_t *mctx) { } if ((instance = isc_mem_allocate(mctx, namelen + 1)) == NULL) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "named_smf_get_instance memory " + UNEXPECTED_ERROR("named_smf_get_instance memory " "allocation failed: %s", isc_result_totext(ISC_R_NOMEMORY)); scf_handle_destroy(h); @@ -1444,8 +1440,7 @@ named_smf_get_instance(char **ins_name, int debug, isc_mem_t *mctx) { if (scf_myname(h, instance, namelen + 1) == -1) { if (debug) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "scf_myname() failed: %s", + UNEXPECTED_ERROR("scf_myname() failed: %s", scf_strerror(scf_error())); } scf_handle_destroy(h); @@ -1552,8 +1547,7 @@ main(int argc, char *argv[]) { result = named_smf_get_instance(&instance, 1, named_g_mctx); if (result == ISC_R_SUCCESS && instance != NULL) { if (smf_disable_instance(instance, 0) != 0) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "smf_disable_instance() " + UNEXPECTED_ERROR("smf_disable_instance() " "failed for %s : %s", instance, scf_strerror(scf_error())); diff --git a/bin/named/server.c b/bin/named/server.c index 8c948ef6f4..7f4bb94e67 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4765,8 +4765,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, maps, AF_INET6, &dispatch6, &dscp6, (ISC_LIST_PREV(view, link) == NULL))); if (dispatch4 == NULL && dispatch6 == NULL) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "unable to obtain either an IPv4 or" + UNEXPECTED_ERROR("unable to obtain either an IPv4 or" " an IPv6 dispatch"); result = ISC_R_UNEXPECTED; goto cleanup; diff --git a/doc/dev/unexpected b/doc/dev/unexpected index ffa4b2a9b9..bb9f00a8b5 100644 --- a/doc/dev/unexpected +++ b/doc/dev/unexpected @@ -37,8 +37,7 @@ not, we call UNEXPECTED_ERROR(). E.g. void foo() { if (some_unix_thang() < 0) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "some_unix_thang() failed: %s", + UNEXPECTED_ERROR("some_unix_thang() failed: %s", strerror(errno)); return (ISC_R_UNEXPECTED); } diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 51b9cfecdf..eedf3afc64 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -471,8 +471,7 @@ cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr, if (taskmgr != NULL) { result = isc_task_create(taskmgr, &cleaner->task, 0); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_task_create() failed: %s", + UNEXPECTED_ERROR("isc_task_create() failed: %s", isc_result_totext(result)); result = ISC_R_UNEXPECTED; goto cleanup_iterator; @@ -534,8 +533,7 @@ begin_cleaning(cache_cleaner_t *cleaner) { * so there is nothing to be cleaned. */ if (result != ISC_R_NOMORE && cleaner->iterator != NULL) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "cache cleaner: " + UNEXPECTED_ERROR("cache cleaner: " "dns_dbiterator_first() failed: %s", isc_result_totext(result)); dns_dbiterator_destroy(&cleaner->iterator); @@ -674,10 +672,8 @@ incremental_cleaning_action(isc_task_t *task, isc_event_t *event) { result = dns_dbiterator_current(cleaner->iterator, &node, NULL); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "cache cleaner: " - "dns_dbiterator_current() " - "failed: %s", + UNEXPECTED_ERROR("cache cleaner: " + "dns_dbiterator_current() failed: %s", isc_result_totext(result)); end_cleaning(cleaner, event); @@ -703,8 +699,7 @@ incremental_cleaning_action(isc_task_t *task, isc_event_t *event) { * keep trying to clean it, otherwise stop cleaning. */ if (result != ISC_R_NOMORE) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "cache cleaner: " + UNEXPECTED_ERROR("cache cleaner: " "dns_dbiterator_next() " "failed: %s", isc_result_totext(result)); @@ -779,8 +774,7 @@ dns_cache_clean(dns_cache_t *cache, isc_stdtime_t now) { */ result = dns_db_expirenode(cache->db, node, now); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "cache cleaner: dns_db_expirenode() " + UNEXPECTED_ERROR("cache cleaner: dns_db_expirenode() " "failed: %s", isc_result_totext(result)); /* diff --git a/lib/dns/diff.c b/lib/dns/diff.c index 74d4689fee..1ec38b85e7 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -176,8 +176,7 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) { { ISC_LIST_UNLINK(diff->tuples, ot, link); if ((*tuplep)->op == ot->op) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "unexpected non-minimal diff"); + UNEXPECTED_ERROR("unexpected non-minimal diff"); } else { dns_difftuple_free(tuplep); } diff --git a/lib/dns/journal.c b/lib/dns/journal.c index ff8e37d69b..54467dd18f 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -168,7 +168,7 @@ dns_db_createsoatuple(dns_db_t *db, dns_dbversion_t *ver, isc_mem_t *mctx, freenode: dns_db_detachnode(db, &node); nonode: - UNEXPECTED_ERROR(__FILE__, __LINE__, "missing SOA"); + UNEXPECTED_ERROR("missing SOA"); return (result); } diff --git a/lib/dns/master.c b/lib/dns/master.c index da2c1ee735..b433f146c8 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -445,8 +445,7 @@ loadctx_destroy(dns_loadctx_t *lctx) { if (lctx->f != NULL) { isc_result_t result = isc_stdio_close(lctx->f); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_stdio_close() failed: %s", + UNEXPECTED_ERROR("isc_stdio_close() failed: %s", isc_result_totext(result)); } } @@ -1281,8 +1280,7 @@ load_text(dns_loadctx_t *lctx) { } dump_time = (isc_stdtime_t)dump_time64; if (dump_time != dump_time64) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "%s: %s:%lu: $DATE " + UNEXPECTED_ERROR("%s: %s:%lu: $DATE " "outside epoch", "dns_master_load", source, line); @@ -1290,8 +1288,7 @@ load_text(dns_loadctx_t *lctx) { goto insist_and_cleanup; } if (dump_time > current_time) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "%s: %s:%lu: " + UNEXPECTED_ERROR("%s: %s:%lu: " "$DATE in future, " "using current date", "dns_master_load", @@ -1570,8 +1567,7 @@ load_text(dns_loadctx_t *lctx) { ictx->drop = false; } } else { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "%s:%lu: isc_lex_gettoken() returned " + UNEXPECTED_ERROR("%s:%lu: isc_lex_gettoken() returned " "unexpected token type (%d)", source, line, token.type); result = ISC_R_UNEXPECTED; @@ -1663,8 +1659,7 @@ load_text(dns_loadctx_t *lctx) { } if (token.type != isc_tokentype_string) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_lex_gettoken() returned " + UNEXPECTED_ERROR("isc_lex_gettoken() returned " "unexpected token type"); result = ISC_R_UNEXPECTED; if (MANYERRS(lctx, result)) { @@ -1685,8 +1680,7 @@ load_text(dns_loadctx_t *lctx) { } if (token.type != isc_tokentype_string) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_lex_gettoken() returned " + UNEXPECTED_ERROR("isc_lex_gettoken() returned " "unexpected token type"); result = ISC_R_UNEXPECTED; if (MANYERRS(lctx, result)) { @@ -2273,8 +2267,7 @@ load_header(dns_loadctx_t *lctx) { result = isc_stdio_read(data, 1, commonlen, lctx->f, NULL); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_stdio_read failed: %s", + UNEXPECTED_ERROR("isc_stdio_read failed: %s", isc_result_totext(result)); return (result); } @@ -2306,8 +2299,7 @@ load_header(dns_loadctx_t *lctx) { result = isc_stdio_read(data + commonlen, 1, remainder, lctx->f, NULL); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_stdio_read failed: %s", + UNEXPECTED_ERROR("isc_stdio_read failed: %s", isc_result_totext(result)); return (result); } @@ -2332,8 +2324,7 @@ openfile_raw(dns_loadctx_t *lctx, const char *master_file) { result = isc_stdio_open(master_file, "rb", &lctx->f); if (result != ISC_R_SUCCESS && result != ISC_R_FILENOTFOUND) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_stdio_open() failed: %s", + UNEXPECTED_ERROR("isc_stdio_open() failed: %s", isc_result_totext(result)); } diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 72ef134f27..7ab6021aca 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -857,8 +857,7 @@ dns_rdataset_totext(dns_rdataset_t *rdataset, const dns_name_t *owner_name, isc_result_t result; result = totext_ctx_init(&dns_master_style_debug, NULL, &ctx); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "could not set master file style"); + UNEXPECTED_ERROR("could not set master file style"); return (ISC_R_UNEXPECTED); } @@ -890,8 +889,7 @@ dns_master_rdatasettotext(const dns_name_t *owner_name, isc_result_t result; result = totext_ctx_init(style, indent, &ctx); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "could not set master file style"); + UNEXPECTED_ERROR("could not set master file style"); return (ISC_R_UNEXPECTED); } @@ -907,8 +905,7 @@ dns_master_questiontotext(const dns_name_t *owner_name, isc_result_t result; result = totext_ctx_init(style, NULL, &ctx); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "could not set master file style"); + UNEXPECTED_ERROR("could not set master file style"); return (ISC_R_UNEXPECTED); } @@ -983,8 +980,7 @@ dump_rdataset(isc_mem_t *mctx, const dns_name_t *name, dns_rdataset_t *rdataset, result = isc_stdio_write(r.base, 1, (size_t)r.length, f, NULL); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "master file write failed: %s", + UNEXPECTED_ERROR("master file write failed: %s", isc_result_totext(result)); return (result); } @@ -1266,8 +1262,7 @@ restart: result = isc_stdio_write(r.base, 1, (size_t)r.length, f, NULL); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "raw master file write failed: %s", + UNEXPECTED_ERROR("raw master file write failed: %s", isc_result_totext(result)); return (result); } @@ -1582,8 +1577,7 @@ dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, result = totext_ctx_init(style, NULL, &dctx->tctx); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "could not set master file style"); + UNEXPECTED_ERROR("could not set master file style"); goto cleanup; } @@ -1973,8 +1967,7 @@ dns_master_dumpnodetostream(isc_mem_t *mctx, dns_db_t *db, result = totext_ctx_init(style, NULL, &ctx); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "could not set master file style"); + UNEXPECTED_ERROR("could not set master file style"); return (ISC_R_UNEXPECTED); } diff --git a/lib/dns/name.c b/lib/dns/name.c index d615848321..b8a9ec46f0 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -1058,8 +1058,7 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source, } break; default: - FATAL_ERROR(__FILE__, __LINE__, "Unexpected state %d", - state); + FATAL_ERROR("Unexpected state %d", state); /* Does not return. */ } } @@ -1279,8 +1278,7 @@ dns_name_totext2(const dns_name_t *name, unsigned int options, count--; } } else { - FATAL_ERROR(__FILE__, __LINE__, - "Unexpected label type %02x", count); + FATAL_ERROR("Unexpected label type %02x", count); UNREACHABLE(); } @@ -1402,8 +1400,7 @@ dns_name_tofilenametext(const dns_name_t *name, bool omit_final_dot, count--; } } else { - FATAL_ERROR(__FILE__, __LINE__, - "Unexpected label type %02x", count); + FATAL_ERROR("Unexpected label type %02x", count); UNREACHABLE(); } @@ -1659,8 +1656,7 @@ dns_name_fromwire(dns_name_t *name, isc_buffer_t *source, dns_decompress_t dctx, state = fw_start; break; default: - FATAL_ERROR(__FILE__, __LINE__, "Unknown state %d", - state); + FATAL_ERROR("Unknown state %d", state); /* Does not return. */ } } diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index e9a0566464..b35ad05cc3 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -4426,7 +4426,7 @@ zone_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, UNUSED(rdataset); UNUSED(sigrdataset); - FATAL_ERROR(__FILE__, __LINE__, "zone_findzonecut() called!"); + FATAL_ERROR("zone_findzonecut() called!"); UNREACHABLE(); return (ISC_R_NOTIMPLEMENTED); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 756878729d..5d1e7291ed 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4880,8 +4880,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, if (!dns_name_issubdomain(fctx->name, fctx->domain)) { dns_name_format(fctx->domain, buf, sizeof(buf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, - "'%s' is not subdomain of '%s'", fctx->info, + UNEXPECTED_ERROR("'%s' is not subdomain of '%s'", fctx->info, buf); result = ISC_R_UNEXPECTED; goto cleanup_fcount; @@ -4897,8 +4896,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, res->query_timeout % 1000 * 1000000); iresult = isc_time_nowplusinterval(&fctx->expires, &interval); if (iresult != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_time_nowplusinterval: %s", + UNEXPECTED_ERROR("isc_time_nowplusinterval: %s", isc_result_totext(iresult)); result = ISC_R_UNEXPECTED; goto cleanup_qmessage; @@ -4925,8 +4923,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, iresult = isc_time_nowplusinterval(&fctx->expires_try_stale, &interval); if (iresult != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_time_nowplusinterval: %s", + UNEXPECTED_ERROR("isc_time_nowplusinterval: %s", isc_result_totext(iresult)); result = ISC_R_UNEXPECTED; goto cleanup_timer; diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 535e9e0872..8992c99cdc 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -207,7 +207,7 @@ dns_rpz_type2str(dns_rpz_type_t type) { case DNS_RPZ_TYPE_BAD: break; } - FATAL_ERROR(__FILE__, __LINE__, "impossible rpz type %d", type); + FATAL_ERROR("impossible rpz type %d", type); return ("impossible"); } diff --git a/lib/dns/view.c b/lib/dns/view.c index 5a8d1c8072..22f43f757a 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -138,8 +138,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, view->zonetable = NULL; result = dns_zt_create(mctx, rdclass, &view->zonetable); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "dns_zt_create() failed: %s", + UNEXPECTED_ERROR("dns_zt_create() failed: %s", isc_result_totext(result)); result = ISC_R_UNEXPECTED; goto cleanup_mutex; @@ -147,8 +146,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, result = dns_fwdtable_create(mctx, &view->fwdtable); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "dns_fwdtable_create() failed: %s", + UNEXPECTED_ERROR("dns_fwdtable_create() failed: %s", isc_result_totext(result)); result = ISC_R_UNEXPECTED; goto cleanup_zt; diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 7e7355598c..35b3455266 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -5240,8 +5240,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, break; default: - UNEXPECTED_ERROR(__FILE__, __LINE__, "unexpected zone type %d", - zone->type); + UNEXPECTED_ERROR("unexpected zone type %d", zone->type); result = ISC_R_UNEXPECTED; goto cleanup; } diff --git a/lib/isc/condition.c b/lib/isc/condition.c index 0573c37e2f..c81f3213b5 100644 --- a/lib/isc/condition.c +++ b/lib/isc/condition.c @@ -62,7 +62,6 @@ isc__condition_waituntil(pthread_cond_t *c, pthread_mutex_t *m, isc_time_t *t) { } while (presult == EINTR); strerror_r(presult, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, - "pthread_cond_timedwait() returned %s", strbuf); + UNEXPECTED_ERROR("pthread_cond_timedwait() returned %s", strbuf); return (ISC_R_UNEXPECTED); } diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 9f7098c8c9..b93ddec2a4 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -315,10 +315,10 @@ mock_assert(const int result, const char *const expression, #include /* Contractual promise. */ #include /* for ISC_STRERRORSIZE */ -/*% Unexpected Error */ -#define UNEXPECTED_ERROR isc_error_unexpected -/*% Fatal Error */ -#define FATAL_ERROR isc_error_fatal +#define UNEXPECTED_ERROR(...) \ + isc_error_unexpected(__FILE__, __LINE__, __VA_ARGS__) + +#define FATAL_ERROR(...) isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__) #ifdef UNIT_TESTING @@ -329,7 +329,6 @@ mock_assert(const int result, const char *const expression, #else /* UNIT_TESTING */ -/*% Runtime Check */ #define RUNTIME_CHECK(cond) ISC_ERROR_RUNTIMECHECK(cond) #endif /* UNIT_TESTING */ diff --git a/lib/isc/interfaceiter.c b/lib/isc/interfaceiter.c index abb16f4b6d..566c5bd465 100644 --- a/lib/isc/interfaceiter.c +++ b/lib/isc/interfaceiter.c @@ -202,8 +202,7 @@ isc_interfaceiter_create(isc_mem_t *mctx, isc_interfaceiter_t **iterp) { if (getifaddrs(&iter->ifaddrs) < 0) { strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, - "getting interface addresses: getifaddrs: %s", + UNEXPECTED_ERROR("getting interface addresses: getifaddrs: %s", strbuf); result = ISC_R_UNEXPECTED; goto failure; diff --git a/lib/isc/lex.c b/lib/isc/lex.c index 50218b78b3..63b9b79d34 100644 --- a/lib/isc/lex.c +++ b/lib/isc/lex.c @@ -902,8 +902,7 @@ isc_lex_gettoken(isc_lex_t *lex, unsigned int options, isc_token_t *tokenp) { remaining--; break; default: - FATAL_ERROR(__FILE__, __LINE__, "Unexpected state %d", - state); + FATAL_ERROR("Unexpected state %d", state); } } while (!done); diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 39ce4cec17..17040c7c9b 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -1249,10 +1249,9 @@ isc__mempool_destroy(isc_mempool_t **restrict mpctxp FLARG) { #endif if (mpctx->allocated > 0) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_mempool_destroy(): mempool %s " - "leaked memory", - mpctx->name); + UNEXPECTED_ERROR( + "isc_mempool_destroy(): mempool %s leaked memory", + mpctx->name); } REQUIRE(mpctx->allocated == 0); diff --git a/lib/isc/net.c b/lib/isc/net.c index be75fb480b..4b84f2c072 100644 --- a/lib/isc/net.c +++ b/lib/isc/net.c @@ -142,8 +142,7 @@ try_proto(int domain) { return (ISC_R_NOTFOUND); default: strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, - "socket() failed: %s", strbuf); + UNEXPECTED_ERROR("socket() failed: %s", strbuf); return (ISC_R_UNEXPECTED); } } @@ -241,8 +240,7 @@ try_ipv6only(void) { s = socket(PF_INET6, SOCK_STREAM, 0); if (s == -1) { strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, "socket() failed: %s", - strbuf); + UNEXPECTED_ERROR("socket() failed: %s", strbuf); ipv6only_result = ISC_R_UNEXPECTED; return; } @@ -259,8 +257,7 @@ try_ipv6only(void) { s = socket(PF_INET6, SOCK_DGRAM, 0); if (s == -1) { strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, "socket() failed: %s", - strbuf); + UNEXPECTED_ERROR("socket() failed: %s", strbuf); ipv6only_result = ISC_R_UNEXPECTED; return; } @@ -302,8 +299,7 @@ try_ipv6pktinfo(void) { s = socket(PF_INET6, SOCK_DGRAM, IPPROTO_UDP); if (s == -1) { strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, "socket() failed: %s", - strbuf); + UNEXPECTED_ERROR("socket() failed: %s", strbuf); ipv6pktinfo_result = ISC_R_UNEXPECTED; return; } @@ -422,13 +418,12 @@ make_nonblock(int fd) { if (ret == -1) { strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, #ifdef USE_FIONBIO_IOCTL - "ioctl(%d, FIONBIO, &on): %s", fd, + UNEXPECTED_ERROR("ioctl(%d, FIONBIO, &on): %s", fd, strbuf); #else /* ifdef USE_FIONBIO_IOCTL */ - "fcntl(%d, F_SETFL, %d): %s", fd, flags, -#endif /* ifdef USE_FIONBIO_IOCTL */ + UNEXPECTED_ERROR("fcntl(%d, F_SETFL, %d): %s", fd, flags, strbuf); +#endif /* ifdef USE_FIONBIO_IOCTL */ return (ISC_R_UNEXPECTED); } @@ -509,7 +504,6 @@ cmsgsend(int s, int level, int type, struct addrinfo *res) { if (sendmsg(s, &msg, 0) < 0) { int debug = ISC_LOG_DEBUG(10); - const char *typestr; switch (errno) { #ifdef ENOPROTOOPT case ENOPROTOOPT: @@ -529,11 +523,10 @@ cmsgsend(int s, int level, int type, struct addrinfo *res) { ISC_LOGMODULE_SOCKET, ISC_LOG_DEBUG(10), "sendmsg: %s", strbuf); } else { - typestr = (type == IP_TOS) ? "IP_TOS" : "IPV6_TCLASS"; - UNEXPECTED_ERROR(__FILE__, __LINE__, - "probing " - "sendmsg() with %s=%02x failed: %s", - typestr, dscp, strbuf); + UNEXPECTED_ERROR( + "probing sendmsg() with %s=%02x failed: %s", + (type == IP_TOS) ? "IP_TOS" : "IPV6_TCLASS", + dscp, strbuf); } return (false); } diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c index ec3afa74d0..8f1956e5c4 100644 --- a/lib/isc/rwlock.c +++ b/lib/isc/rwlock.c @@ -224,8 +224,7 @@ isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota, rwl->readers_waiting = 0; atomic_init(&rwl->write_granted, 0); if (read_quota != 0) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "read quota is not supported"); + UNEXPECTED_ERROR("read quota is not supported"); } if (write_quota == 0) { write_quota = RWLOCK_DEFAULT_WRITE_QUOTA; diff --git a/lib/isc/sockaddr.c b/lib/isc/sockaddr.c index 85b3d28439..038e3ec7c4 100644 --- a/lib/isc/sockaddr.c +++ b/lib/isc/sockaddr.c @@ -234,8 +234,7 @@ isc_sockaddr_hash(const isc_sockaddr_t *sockaddr, bool address_only) { p = ntohs(sockaddr->type.sin6.sin6_port); break; default: - UNEXPECTED_ERROR(__FILE__, __LINE__, - "unknown address family: %d", + UNEXPECTED_ERROR("unknown address family: %d", (int)sockaddr->type.sa.sa_family); s = (const unsigned char *)&sockaddr->type; length = sockaddr->length; @@ -341,7 +340,7 @@ isc_sockaddr_pf(const isc_sockaddr_t *sockaddr) { case AF_INET6: return (PF_INET6); default: - FATAL_ERROR(__FILE__, __LINE__, "unknown address family: %d", + FATAL_ERROR("unknown address family: %d", (int)sockaddr->type.sa.sa_family); } #endif /* if (AF_INET == PF_INET && AF_INET6 == PF_INET6) */ @@ -380,7 +379,7 @@ isc_sockaddr_setport(isc_sockaddr_t *sockaddr, in_port_t port) { sockaddr->type.sin6.sin6_port = htons(port); break; default: - FATAL_ERROR(__FILE__, __LINE__, "unknown address family: %d", + FATAL_ERROR("unknown address family: %d", (int)sockaddr->type.sa.sa_family); } } @@ -397,7 +396,7 @@ isc_sockaddr_getport(const isc_sockaddr_t *sockaddr) { port = ntohs(sockaddr->type.sin6.sin6_port); break; default: - FATAL_ERROR(__FILE__, __LINE__, "unknown address family: %d", + FATAL_ERROR("unknown address family: %d", (int)sockaddr->type.sa.sa_family); } diff --git a/lib/isc/time.c b/lib/isc/time.c index 26077befe8..4c01132adb 100644 --- a/lib/isc/time.c +++ b/lib/isc/time.c @@ -94,7 +94,7 @@ time_now(isc_time_t *t, clockid_t clock) { if (clock_gettime(clock, &ts) == -1) { char strbuf[ISC_STRERRORSIZE]; strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, "%s", strbuf); + UNEXPECTED_ERROR("%s", strbuf); return (ISC_R_UNEXPECTED); } @@ -138,7 +138,7 @@ isc_time_nowplusinterval(isc_time_t *t, const isc_interval_t *i) { if (clock_gettime(CLOCKSOURCE, &ts) == -1) { char strbuf[ISC_STRERRORSIZE]; strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR(__FILE__, __LINE__, "%s", strbuf); + UNEXPECTED_ERROR("%s", strbuf); return (ISC_R_UNEXPECTED); } diff --git a/lib/isc/tls.c b/lib/isc/tls.c index d7302c2143..f38182a0db 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -209,8 +209,7 @@ isc__tls_initialize(void) { /* Protect ourselves against unseeded PRNG */ if (RAND_status() != 1) { - FATAL_ERROR(__FILE__, __LINE__, - "OpenSSL pseudorandom number generator " + FATAL_ERROR("OpenSSL pseudorandom number generator " "cannot be initialized (see the `PRNG not " "seeded' message in the OpenSSL FAQ)"); } diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index a17f1f35f7..dce611b92d 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -1319,8 +1319,7 @@ do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { continue; } if (result != ISC_R_NOMORE) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "interface iteration failed: %s", + UNEXPECTED_ERROR("interface iteration failed: %s", isc_result_totext(result)); } else { result = ((tried_listening && all_addresses_in_use) diff --git a/lib/ns/sortlist.c b/lib/ns/sortlist.c index 3e6a3517e9..2aada90b19 100644 --- a/lib/ns/sortlist.c +++ b/lib/ns/sortlist.c @@ -173,10 +173,9 @@ ns_sortlist_byaddrsetup(dns_acl_t *sortlist_acl, dns_aclenv_t *env, *orderp = NULL; break; default: - UNEXPECTED_ERROR(__FILE__, __LINE__, - "unexpected return from ns_sortlist_setup(): " - "%d", - sortlisttype); + UNEXPECTED_ERROR( + "unexpected return from ns_sortlist_setup(): %d", + sortlisttype); break; } } diff --git a/lib/ns/update.c b/lib/ns/update.c index ad7c561c9d..51cedf60e3 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -2745,10 +2745,9 @@ update_action(isc_task_t *task, isc_event_t *event) { /* "temp += rr;" */ result = temp_append(&temp, name, &rdata); if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "temp entry creation failed: " - "%s", - isc_result_totext(result)); + UNEXPECTED_ERROR( + "temp entry creation failed: %s", + isc_result_totext(result)); FAIL(ISC_R_UNEXPECTED); } } else { From a34a2784b191f79d7b55b8f61560539655f68ed3 Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Fri, 14 Oct 2022 17:18:07 +0100 Subject: [PATCH 2/4] De-duplicate some calls to strerror_r() Specifically, when reporting an unexpected or fatal error. --- lib/isc/condition.c | 4 +-- lib/isc/include/isc/error.h | 4 --- lib/isc/include/isc/util.h | 34 +++++++++++++++--------- lib/isc/include/isc/uv.h | 7 +++-- lib/isc/loop.c | 8 ++---- lib/isc/net.c | 52 +++++++++++++------------------------ lib/isc/netmgr/netmgr.c | 9 +++---- lib/isc/stdtime.c | 5 +--- lib/isc/time.c | 8 ++---- 9 files changed, 53 insertions(+), 78 deletions(-) diff --git a/lib/isc/condition.c b/lib/isc/condition.c index c81f3213b5..baf2808a6b 100644 --- a/lib/isc/condition.c +++ b/lib/isc/condition.c @@ -26,7 +26,6 @@ isc__condition_waituntil(pthread_cond_t *c, pthread_mutex_t *m, isc_time_t *t) { int presult; isc_result_t result; struct timespec ts; - char strbuf[ISC_STRERRORSIZE]; REQUIRE(c != NULL && m != NULL && t != NULL); @@ -61,7 +60,6 @@ isc__condition_waituntil(pthread_cond_t *c, pthread_mutex_t *m, isc_time_t *t) { } } while (presult == EINTR); - strerror_r(presult, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("pthread_cond_timedwait() returned %s", strbuf); + UNEXPECTED_SYSERROR(presult, "pthread_cond_timedwait()"); return (ISC_R_UNEXPECTED); } diff --git a/lib/isc/include/isc/error.h b/lib/isc/include/isc/error.h index ecebab60a2..96552bcb78 100644 --- a/lib/isc/include/isc/error.h +++ b/lib/isc/include/isc/error.h @@ -44,8 +44,4 @@ isc_error_fatal(const char *, int, const char *, ...) ISC_FORMAT_PRINTF(3, 4); noreturn void isc_error_runtimecheck(const char *, int, const char *); -#define ISC_ERROR_RUNTIMECHECK(cond) \ - ((void)((cond) || \ - ((isc_error_runtimecheck)(__FILE__, __LINE__, #cond), 0))) - ISC_LANG_ENDDECLS diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index b93ddec2a4..38b025b392 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -320,16 +320,30 @@ mock_assert(const int result, const char *const expression, #define FATAL_ERROR(...) isc_error_fatal(__FILE__, __LINE__, __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 UNEXPECTED_SYSERROR(err, ...) \ + REPORT_SYSERROR(isc_error_unexpected, err, __VA_ARGS__) + +#define FATAL_SYSERROR(err, ...) \ + REPORT_SYSERROR(isc_error_fatal, err, __VA_ARGS__) + #ifdef UNIT_TESTING -#define RUNTIME_CHECK(expression) \ - ((!(expression)) \ - ? (mock_assert(0, #expression, __FILE__, __LINE__), abort()) \ - : (void)0) +#define RUNTIME_CHECK(cond) \ + ((cond) ? (void)0 \ + : (mock_assert(0, #cond, __FILE__, __LINE__), abort())) #else /* UNIT_TESTING */ -#define RUNTIME_CHECK(cond) ISC_ERROR_RUNTIMECHECK(cond) +#define RUNTIME_CHECK(cond) \ + ((cond) ? (void)0 : isc_error_runtimecheck(__FILE__, __LINE__, #cond)) #endif /* UNIT_TESTING */ @@ -337,13 +351,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) { \ - char _strerrorbuf[ISC_STRERRORSIZE]; \ - strerror_r(ret, _strerrorbuf, sizeof(_strerrorbuf)); \ - isc_error_fatal(__FILE__, __LINE__, \ - "%s(): %s() failed with error %d (%s)", \ - __func__, #func, ret, _strerrorbuf); \ +#define PTHREADS_RUNTIME_CHECK(func, ret) \ + if ((ret) != 0) { \ + FATAL_SYSERROR(ret, "%s(): %s()", __func__, #func); \ } /*% diff --git a/lib/isc/include/isc/uv.h b/lib/isc/include/isc/uv.h index 8f80b65f82..541f697b1b 100644 --- a/lib/isc/include/isc/uv.h +++ b/lib/isc/include/isc/uv.h @@ -46,10 +46,9 @@ * These are used with all versions of libuv: */ -#define UV_RUNTIME_CHECK(func, ret) \ - if (ret != 0) { \ - isc_error_fatal(__FILE__, __LINE__, "%s failed: %s\n", #func, \ - uv_strerror(ret)); \ +#define UV_RUNTIME_CHECK(func, ret) \ + if (ret != 0) { \ + FATAL_ERROR("%s failed: %s\n", #func, uv_strerror(ret)); \ } #define isc_uverr2result(x) \ diff --git a/lib/isc/loop.c b/lib/isc/loop.c index df414eed7c..52817be01c 100644 --- a/lib/isc/loop.c +++ b/lib/isc/loop.c @@ -45,14 +45,10 @@ static void ignore_signal(int sig, void (*handler)(int)) { - struct sigaction sa; + struct sigaction sa = { .sa_handler = handler }; - sa = (struct sigaction){ .sa_handler = handler }; if (sigfillset(&sa.sa_mask) != 0 || sigaction(sig, &sa, NULL) < 0) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(errno, strbuf, sizeof(strbuf)); - isc_error_fatal(__FILE__, __LINE__, "%s() %d setup: %s", - __func__, sig, strbuf); + FATAL_SYSERROR(errno, "ignore_signal(%d)", sig); } } diff --git a/lib/isc/net.c b/lib/isc/net.c index 4b84f2c072..89a57551fe 100644 --- a/lib/isc/net.c +++ b/lib/isc/net.c @@ -122,7 +122,6 @@ static isc_result_t try_proto(int domain) { int s; isc_result_t result = ISC_R_SUCCESS; - char strbuf[ISC_STRERRORSIZE]; s = socket(domain, SOCK_STREAM, 0); if (s == -1) { @@ -141,8 +140,7 @@ try_proto(int domain) { #endif /* ifdef EINVAL */ return (ISC_R_NOTFOUND); default: - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("socket() failed: %s", strbuf); + UNEXPECTED_SYSERROR(errno, "socket()"); return (ISC_R_UNEXPECTED); } } @@ -222,7 +220,6 @@ static void try_ipv6only(void) { #ifdef IPV6_V6ONLY int s, on; - char strbuf[ISC_STRERRORSIZE]; #endif /* ifdef IPV6_V6ONLY */ isc_result_t result; @@ -239,8 +236,7 @@ try_ipv6only(void) { /* check for TCP sockets */ s = socket(PF_INET6, SOCK_STREAM, 0); if (s == -1) { - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("socket() failed: %s", strbuf); + UNEXPECTED_SYSERROR(errno, "socket()"); ipv6only_result = ISC_R_UNEXPECTED; return; } @@ -256,8 +252,7 @@ try_ipv6only(void) { /* check for UDP sockets */ s = socket(PF_INET6, SOCK_DGRAM, 0); if (s == -1) { - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("socket() failed: %s", strbuf); + UNEXPECTED_SYSERROR(errno, "socket()"); ipv6only_result = ISC_R_UNEXPECTED; return; } @@ -285,7 +280,6 @@ initialize_ipv6only(void) { static void try_ipv6pktinfo(void) { int s, on; - char strbuf[ISC_STRERRORSIZE]; isc_result_t result; int optname; @@ -298,8 +292,7 @@ try_ipv6pktinfo(void) { /* we only use this for UDP sockets */ s = socket(PF_INET6, SOCK_DGRAM, IPPROTO_UDP); if (s == -1) { - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("socket() failed: %s", strbuf); + UNEXPECTED_SYSERROR(errno, "socket()"); ipv6pktinfo_result = ISC_R_UNEXPECTED; return; } @@ -405,11 +398,10 @@ static isc_result_t make_nonblock(int fd) { int ret; int flags; - char strbuf[ISC_STRERRORSIZE]; -#ifdef USE_FIONBIO_IOCTL - int on = 1; - ret = ioctl(fd, FIONBIO, (char *)&on); +#ifdef USE_FIONBIO_IOCTL + flags = 1; + ret = ioctl(fd, FIONBIO, (char *)&flags); #else /* ifdef USE_FIONBIO_IOCTL */ flags = fcntl(fd, F_GETFL, 0); flags |= O_NONBLOCK; @@ -417,14 +409,11 @@ make_nonblock(int fd) { #endif /* ifdef USE_FIONBIO_IOCTL */ if (ret == -1) { - strerror_r(errno, strbuf, sizeof(strbuf)); #ifdef USE_FIONBIO_IOCTL - UNEXPECTED_ERROR("ioctl(%d, FIONBIO, &on): %s", fd, strbuf); -#else /* ifdef USE_FIONBIO_IOCTL */ - UNEXPECTED_ERROR("fcntl(%d, F_SETFL, %d): %s", fd, flags, - strbuf); -#endif /* ifdef USE_FIONBIO_IOCTL */ - + UNEXPECTED_SYSERROR(errno, "ioctl(%d, FIONBIO, &on)", fd); +#else + UNEXPECTED_SYSERROR(errno, "fcntl(%d, F_SETFL, %d)", fd, flags); +#endif return (ISC_R_UNEXPECTED); } @@ -503,7 +492,6 @@ cmsgsend(int s, int level, int type, struct addrinfo *res) { } if (sendmsg(s, &msg, 0) < 0) { - int debug = ISC_LOG_DEBUG(10); switch (errno) { #ifdef ENOPROTOOPT case ENOPROTOOPT: @@ -513,20 +501,17 @@ cmsgsend(int s, int level, int type, struct addrinfo *res) { #endif /* ifdef EOPNOTSUPP */ case EINVAL: case EPERM: - break; - default: - debug = ISC_LOG_NOTICE; - } - strerror_r(errno, strbuf, sizeof(strbuf)); - if (debug != ISC_LOG_NOTICE) { + strerror_r(errno, strbuf, sizeof(strbuf)); isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_SOCKET, ISC_LOG_DEBUG(10), "sendmsg: %s", strbuf); - } else { - UNEXPECTED_ERROR( - "probing sendmsg() with %s=%02x failed: %s", + break; + default: + UNEXPECTED_SYSERROR( + errno, "probing sendmsg() with %s=%02x failed", (type == IP_TOS) ? "IP_TOS" : "IPV6_TCLASS", - dscp, strbuf); + dscp); + break; } return (false); } @@ -586,7 +571,6 @@ try_dscp_v4(void) { } s = socket(res0->ai_family, res0->ai_socktype, res0->ai_protocol); - if (s == -1) { strerror_r(errno, strbuf, sizeof(strbuf)); isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index ac9ba9b695..e33dbbc4ca 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -198,11 +198,10 @@ isc_netmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t **netmgrp) { isc_nm_t *netmgr = NULL; if (uv_version() < MINIMAL_UV_VERSION) { - isc_error_fatal(__FILE__, __LINE__, - "libuv version too old: running with libuv %s " - "when compiled with libuv %s will lead to " - "libuv failures because of unknown flags", - uv_version_string(), UV_VERSION_STRING); + FATAL_ERROR("libuv version too old: running with libuv %s " + "when compiled with libuv %s will lead to " + "libuv failures because of unknown flags", + uv_version_string(), UV_VERSION_STRING); } netmgr = isc_mem_get(mctx, sizeof(*netmgr)); diff --git a/lib/isc/stdtime.c b/lib/isc/stdtime.c index ada19d0ef5..086c0c775b 100644 --- a/lib/isc/stdtime.c +++ b/lib/isc/stdtime.c @@ -41,10 +41,7 @@ isc_stdtime_get(isc_stdtime_t *t) { struct timespec ts; if (clock_gettime(CLOCKSOURCE, &ts) == -1) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(errno, strbuf, sizeof(strbuf)); - isc_error_fatal(__FILE__, __LINE__, "clock_gettime failed: %s", - strbuf); + FATAL_SYSERROR(errno, "clock_gettime()"); } REQUIRE(ts.tv_sec > 0 && ts.tv_nsec >= 0 && ts.tv_nsec < NS_PER_S); diff --git a/lib/isc/time.c b/lib/isc/time.c index 4c01132adb..8570a9439a 100644 --- a/lib/isc/time.c +++ b/lib/isc/time.c @@ -92,9 +92,7 @@ time_now(isc_time_t *t, clockid_t clock) { REQUIRE(t != NULL); if (clock_gettime(clock, &ts) == -1) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("%s", strbuf); + UNEXPECTED_SYSERROR(errno, "clock_gettime()"); return (ISC_R_UNEXPECTED); } @@ -136,9 +134,7 @@ isc_time_nowplusinterval(isc_time_t *t, const isc_interval_t *i) { INSIST(i->nanoseconds < NS_PER_S); if (clock_gettime(CLOCKSOURCE, &ts) == -1) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(errno, strbuf, sizeof(strbuf)); - UNEXPECTED_ERROR("%s", strbuf); + UNEXPECTED_SYSERROR(errno, "clock_gettime()"); return (ISC_R_UNEXPECTED); } From 26ed03a61e0cb6359a985784ffad93630cb2aeff Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Fri, 14 Oct 2022 17:37:47 +0100 Subject: [PATCH 3/4] 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. --- bin/named/main.c | 26 ++++++++++--------- bin/tests/system/dyndb/driver/db.c | 6 ++--- bin/tests/system/dyndb/driver/log.h | 2 -- bin/tests/system/dyndb/driver/syncptr.c | 2 +- lib/isc/error.c | 33 +++++++++++-------------- lib/isc/include/isc/error.h | 14 +++++------ lib/isc/include/isc/util.h | 25 ++++++++++--------- lib/isc/mem.c | 4 +-- 8 files changed, 53 insertions(+), 59 deletions(-) diff --git a/bin/named/main.c b/bin/named/main.c index c928069d91..606c3ea858 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 7c3b57abb5..7641751f79 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 2cb968bf81..375db2b46a 100644 --- a/bin/tests/system/dyndb/driver/log.h +++ b/bin/tests/system/dyndb/driver/log.h @@ -34,8 +34,6 @@ #include -#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 81d98e5d43..5124df32b4 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 336be8432e..051a6c48aa 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 96552bcb78..f4f668203f 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 38b025b392..f24a8da2d5 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 /* 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 17040c7c9b..043a60e246 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); From 2ffb582d2c6a1dc6b889d1ef449edc50edb5ae33 Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Fri, 14 Oct 2022 17:48:43 +0100 Subject: [PATCH 4/4] CHANGES for [GL !6914] [cleanup] Less ceremonial UNEXPECTED_ERROR() and FATAL_ERROR() reporting macros. [GL !6914] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 7b80719474..f5abad47db 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5997. [cleanup] Less ceremonial UNEXPECTED_ERROR() and FATAL_ERROR() + reporting macros. [GL !6914] + 5996. [bug] Fix a couple of bugs in cfg_print_duration(), which could result in generating incomplete duration values when printing the configuration using named-checkconf.