From a6dcdc535cabf6c80815dfd8e38f5d5504c7b5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 23 Jul 2019 17:16:57 -0400 Subject: [PATCH] Replace usage of isc_mem_put+isc_mem_detach with isc_mem_putanddetach Using isc_mem_put(mctx, ...) + isc_mem_detach(mctx) required juggling with the local variables when mctx was part of the freed object. The isc_mem_putanddetach function can handle this case internally, but it wasn't used everywhere. This commit apply the semantic patching plus bit of manual work to replace all such occurrences with proper usage of isc_mem_putanddetach(). --- lib/dns/db.c | 5 +---- lib/dns/dispatch.c | 29 +++++++++++++---------------- lib/dns/dlz.c | 5 +---- lib/dns/forward.c | 6 ++---- lib/dns/master.c | 11 ++++------- lib/dns/request.c | 11 ++--------- lib/dns/sdb.c | 29 +++++++++++------------------ lib/dns/sdlz.c | 11 +++-------- lib/dns/tkey.c | 3 +-- lib/dns/zone.c | 8 ++------ lib/isc/timer.c | 5 +---- lib/isc/unix/socket.c | 11 +++-------- lib/isc/win32/socket.c | 12 ++++-------- 13 files changed, 48 insertions(+), 98 deletions(-) diff --git a/lib/dns/db.c b/lib/dns/db.c index 89a22c1fb6..de33b4b7a1 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -900,7 +900,6 @@ dns_db_register(const char *name, dns_dbcreatefunc_t create, void *driverarg, void dns_db_unregister(dns_dbimplementation_t **dbimp) { dns_dbimplementation_t *imp; - isc_mem_t *mctx; REQUIRE(dbimp != NULL && *dbimp != NULL); @@ -910,9 +909,7 @@ dns_db_unregister(dns_dbimplementation_t **dbimp) { *dbimp = NULL; RWLOCK(&implock, isc_rwlocktype_write); ISC_LIST_UNLINK(implementations, imp, link); - mctx = imp->mctx; - isc_mem_put(mctx, imp, sizeof(dns_dbimplementation_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&imp->mctx, imp, sizeof(dns_dbimplementation_t)); RWUNLOCK(&implock, isc_rwlocktype_write); ENSURE(*dbimp == NULL); } diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index a652ceb56b..d4eaa3baa4 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -1610,16 +1610,12 @@ destroy_mgr_ok(dns_dispatchmgr_t *mgr) { */ static void destroy_mgr(dns_dispatchmgr_t **mgrp) { - isc_mem_t *mctx; dns_dispatchmgr_t *mgr; mgr = *mgrp; *mgrp = NULL; - mctx = mgr->mctx; - mgr->magic = 0; - mgr->mctx = NULL; isc_mutex_destroy(&mgr->lock); mgr->state = 0; @@ -1637,27 +1633,29 @@ destroy_mgr(dns_dispatchmgr_t **mgrp) { isc_mutex_destroy(&mgr->rpool_lock); isc_mutex_destroy(&mgr->depool_lock); - if (mgr->qid != NULL) - qid_destroy(mctx, &mgr->qid); + if (mgr->qid != NULL) { + qid_destroy(mgr->mctx, &mgr->qid); + } isc_mutex_destroy(&mgr->buffer_lock); - if (mgr->blackhole != NULL) + if (mgr->blackhole != NULL) { dns_acl_detach(&mgr->blackhole); + } - if (mgr->stats != NULL) + if (mgr->stats != NULL) { isc_stats_detach(&mgr->stats); + } if (mgr->v4ports != NULL) { - isc_mem_put(mctx, mgr->v4ports, + isc_mem_put(mgr->mctx, mgr->v4ports, mgr->nv4ports * sizeof(in_port_t)); } if (mgr->v6ports != NULL) { - isc_mem_put(mctx, mgr->v6ports, + isc_mem_put(mgr->mctx, mgr->v6ports, mgr->nv6ports * sizeof(in_port_t)); } - isc_mem_put(mctx, mgr, sizeof(dns_dispatchmgr_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(dns_dispatchmgr_t)); } static isc_result_t @@ -1847,8 +1845,7 @@ dns_dispatchmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t **mgrp) isc_mutex_destroy(&mgr->depool_lock); isc_mutex_destroy(&mgr->buffer_lock); isc_mutex_destroy(&mgr->lock); - isc_mem_put(mctx, mgr, sizeof(dns_dispatchmgr_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&mctx, mgr, sizeof(dns_dispatchmgr_t)); return (result); } @@ -2349,7 +2346,6 @@ static void dispatch_free(dns_dispatch_t **dispp) { dns_dispatch_t *disp; dns_dispatchmgr_t *mgr; - int i; REQUIRE(VALID_DISPATCH(*dispp)); disp = *dispp; @@ -2376,8 +2372,9 @@ dispatch_free(dns_dispatch_t **dispp) { qid_destroy(mgr->mctx, &disp->qid); if (disp->port_table != NULL) { - for (i = 0; i < DNS_DISPATCH_PORTTABLESIZE; i++) + for (int i = 0; i < DNS_DISPATCH_PORTTABLESIZE; i++) { INSIST(ISC_LIST_EMPTY(disp->port_table[i])); + } isc_mem_put(mgr->mctx, disp->port_table, sizeof(disp->port_table[0]) * DNS_DISPATCH_PORTTABLESIZE); diff --git a/lib/dns/dlz.c b/lib/dns/dlz.c index ba575ba70c..7a607e1c5f 100644 --- a/lib/dns/dlz.c +++ b/lib/dns/dlz.c @@ -370,7 +370,6 @@ dns_dlzstrtoargv(isc_mem_t *mctx, char *s, void dns_dlzunregister(dns_dlzimplementation_t **dlzimp) { dns_dlzimplementation_t *dlz_imp; - isc_mem_t *mctx; /* Write debugging message to log */ isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, @@ -395,14 +394,12 @@ dns_dlzunregister(dns_dlzimplementation_t **dlzimp) { /* remove the dlz_implementation object from the list */ ISC_LIST_UNLINK(dlz_implementations, dlz_imp, link); - mctx = dlz_imp->mctx; /* * Return the memory back to the available memory pool and * remove it from the memory context. */ - isc_mem_put(mctx, dlz_imp, sizeof(dns_dlzimplementation_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&dlz_imp->mctx, dlz_imp, sizeof(*dlz_imp)); /* Unlock the dlz_implementations list. */ RWUNLOCK(&dlz_implock, isc_rwlocktype_write); diff --git a/lib/dns/forward.c b/lib/dns/forward.c index 712602d297..2ac0b31b95 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -197,7 +197,6 @@ dns_fwdtable_find(dns_fwdtable_t *fwdtable, const dns_name_t *name, void dns_fwdtable_destroy(dns_fwdtable_t **fwdtablep) { dns_fwdtable_t *fwdtable; - isc_mem_t *mctx; REQUIRE(fwdtablep != NULL && VALID_FWDTABLE(*fwdtablep)); @@ -206,9 +205,8 @@ dns_fwdtable_destroy(dns_fwdtable_t **fwdtablep) { dns_rbt_destroy(&fwdtable->table); isc_rwlock_destroy(&fwdtable->rwlock); fwdtable->magic = 0; - mctx = fwdtable->mctx; - isc_mem_put(mctx, fwdtable, sizeof(dns_fwdtable_t)); - isc_mem_detach(&mctx); + + isc_mem_putanddetach(&fwdtable->mctx, fwdtable, sizeof(dns_fwdtable_t)); *fwdtablep = NULL; } diff --git a/lib/dns/master.c b/lib/dns/master.c index dc748585a4..ae6429ef97 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -431,7 +431,6 @@ incctx_destroy(isc_mem_t *mctx, dns_incctx_t *ictx) { static void loadctx_destroy(dns_loadctx_t *lctx) { - isc_mem_t *mctx; isc_result_t result; REQUIRE(DNS_LCTX_VALID(lctx)); @@ -453,13 +452,11 @@ loadctx_destroy(dns_loadctx_t *lctx) { if (lctx->lex != NULL && !lctx->keep_lex) isc_lex_destroy(&lctx->lex); - if (lctx->task != NULL) + if (lctx->task != NULL) { isc_task_detach(&lctx->task); - mctx = NULL; - isc_mem_attach(lctx->mctx, &mctx); - isc_mem_detach(&lctx->mctx); - isc_mem_put(mctx, lctx, sizeof(*lctx)); - isc_mem_detach(&mctx); + } + + isc_mem_putanddetach(&lctx->mctx, lctx, sizeof(*lctx)); } static isc_result_t diff --git a/lib/dns/request.c b/lib/dns/request.c index 729bd248a7..ecde2839f7 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -374,7 +374,6 @@ send_shutdown_events(dns_requestmgr_t *requestmgr) { static void mgr_destroy(dns_requestmgr_t *requestmgr) { int i; - isc_mem_t *mctx; req_log(ISC_LOG_DEBUG(3), "mgr_destroy"); @@ -389,9 +388,7 @@ mgr_destroy(dns_requestmgr_t *requestmgr) { if (requestmgr->dispatchv6 != NULL) dns_dispatch_detach(&requestmgr->dispatchv6); requestmgr->magic = 0; - mctx = requestmgr->mctx; - isc_mem_put(mctx, requestmgr, sizeof(*requestmgr)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&requestmgr->mctx, requestmgr, sizeof(*requestmgr)); } static unsigned int @@ -1420,8 +1417,6 @@ req_sendevent(dns_request_t *request, isc_result_t result) { static void req_destroy(dns_request_t *request) { - isc_mem_t *mctx; - REQUIRE(VALID_REQUEST(request)); req_log(ISC_LOG_DEBUG(3), "req_destroy: request %p", request); @@ -1445,9 +1440,7 @@ req_destroy(dns_request_t *request) { dns_tsigkey_detach(&request->tsigkey); if (request->requestmgr != NULL) requestmgr_detach(&request->requestmgr); - mctx = request->mctx; - isc_mem_put(mctx, request, sizeof(*request)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&request->mctx, request, sizeof(*request)); } /* diff --git a/lib/dns/sdb.c b/lib/dns/sdb.c index c5aff2c5f9..f6bfad0fe8 100644 --- a/lib/dns/sdb.c +++ b/lib/dns/sdb.c @@ -243,7 +243,6 @@ dns_sdb_register(const char *drivername, const dns_sdbmethods_t *methods, void dns_sdb_unregister(dns_sdbimplementation_t **sdbimp) { dns_sdbimplementation_t *imp; - isc_mem_t *mctx; REQUIRE(sdbimp != NULL && *sdbimp != NULL); @@ -251,9 +250,7 @@ dns_sdb_unregister(dns_sdbimplementation_t **sdbimp) { dns_db_unregister(&imp->dbimp); isc_mutex_destroy(&imp->driverlock); - mctx = imp->mctx; - isc_mem_put(mctx, imp, sizeof(dns_sdbimplementation_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&imp->mctx, imp, sizeof(dns_sdbimplementation_t)); *sdbimp = NULL; } @@ -523,11 +520,8 @@ attach(dns_db_t *source, dns_db_t **targetp) { static void destroy(dns_sdb_t *sdb) { - isc_mem_t *mctx; dns_sdbimplementation_t *imp = sdb->implementation; - mctx = sdb->common.mctx; - if (imp->methods->destroy != NULL) { MAYBE_LOCK(sdb); imp->methods->destroy(sdb->zone, imp->driverdata, @@ -535,15 +529,14 @@ destroy(dns_sdb_t *sdb) { MAYBE_UNLOCK(sdb); } - isc_mem_free(mctx, sdb->zone); + isc_mem_free(sdb->common.mctx, sdb->zone); sdb->common.magic = 0; sdb->common.impmagic = 0; - dns_name_free(&sdb->common.origin, mctx); + dns_name_free(&sdb->common.origin, sdb->common.mctx); - isc_mem_put(mctx, sdb, sizeof(dns_sdb_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&sdb->common.mctx, sdb, sizeof(dns_sdb_t)); } static void @@ -1022,12 +1015,12 @@ static isc_result_t createiterator(dns_db_t *db, unsigned int options, dns_dbiterator_t **iteratorp) { dns_sdb_t *sdb = (dns_sdb_t *)db; - sdb_dbiterator_t *sdbiter; - dns_sdbimplementation_t *imp = sdb->implementation; - isc_result_t result; - REQUIRE(VALID_SDB(sdb)); + sdb_dbiterator_t *sdbiter; + isc_result_t result; + dns_sdbimplementation_t *imp = sdb->implementation; + if (imp->methods->allnodes == NULL) return (ISC_R_NOTIMPLEMENTED); @@ -1070,10 +1063,11 @@ findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, isc_stdtime_t now, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { + REQUIRE(VALID_SDBNODE(node)); + dns_rdatalist_t *list; dns_sdbnode_t *sdbnode = (dns_sdbnode_t *)node; - REQUIRE(VALID_SDBNODE(node)); UNUSED(db); UNUSED(version); @@ -1318,8 +1312,7 @@ dns_sdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, cleanup_origin: dns_name_free(&sdb->common.origin, mctx); cleanup_lock: - isc_mem_put(mctx, sdb, sizeof(dns_sdb_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&mctx, sdb, sizeof(dns_sdb_t)); return (result); } diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c index 8586426148..9b12321929 100644 --- a/lib/dns/sdlz.c +++ b/lib/dns/sdlz.c @@ -820,10 +820,10 @@ findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, isc_stdtime_t now, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { + REQUIRE(VALID_SDLZNODE(node)); dns_rdatalist_t *list; dns_sdlznode_t *sdlznode = (dns_sdlznode_t *)node; - REQUIRE(VALID_SDLZNODE(node)); UNUSED(db); UNUSED(version); @@ -2064,15 +2064,13 @@ dns_sdlzregister(const char *drivername, const dns_sdlzmethods_t *methods, * return the memory back to the available memory pool and * remove it from the memory context. */ - isc_mem_put(mctx, imp, sizeof(dns_sdlzimplementation_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&imp->mctx, imp, sizeof(dns_sdlzimplementation_t)); return (result); } void dns_sdlzunregister(dns_sdlzimplementation_t **sdlzimp) { dns_sdlzimplementation_t *imp; - isc_mem_t *mctx; /* Write debugging message to log */ sdlz_log(ISC_LOG_DEBUG(2), "Unregistering SDLZ driver."); @@ -2090,14 +2088,11 @@ dns_sdlzunregister(dns_sdlzimplementation_t **sdlzimp) { /* destroy the driver lock, we don't need it anymore */ isc_mutex_destroy(&imp->driverlock); - mctx = imp->mctx; - /* * return the memory back to the available memory pool and * remove it from the memory context. */ - isc_mem_put(mctx, imp, sizeof(dns_sdlzimplementation_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&imp->mctx, imp, sizeof(dns_sdlzimplementation_t)); *sdlzimp = NULL; } diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 02f57fd0e8..965d6d52d6 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -145,8 +145,7 @@ dns_tkeyctx_destroy(dns_tkeyctx_t **tctxp) { } if (tctx->gsscred != NULL) dst_gssapi_releasecred(&tctx->gsscred); - isc_mem_put(mctx, tctx, sizeof(dns_tkeyctx_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&mctx, tctx, sizeof(dns_tkeyctx_t)); *tctxp = NULL; } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 100477eeb9..15f8e65415 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1094,7 +1094,6 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { */ static void zone_free(dns_zone_t *zone) { - isc_mem_t *mctx = NULL; dns_signing_t *signing; dns_nsec3chain_t *nsec3chain; isc_event_t *setnsec3param_event; @@ -1255,9 +1254,7 @@ zone_free(dns_zone_t *zone) { ZONEDB_DESTROYLOCK(&zone->dblock); isc_mutex_destroy(&zone->lock); zone->magic = 0; - mctx = zone->mctx; - isc_mem_put(mctx, zone, sizeof(*zone)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&zone->mctx, zone, sizeof(*zone)); } /* @@ -10101,8 +10098,7 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { } dns_name_free(keyname, mctx); - isc_mem_put(mctx, kfetch, sizeof(dns_keyfetch_t)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&mctx, kfetch, sizeof(dns_keyfetch_t)); if (secroots != NULL) { dns_keytable_detach(&secroots); diff --git a/lib/isc/timer.c b/lib/isc/timer.c index ddd9ace486..1c3ef3d0e3 100644 --- a/lib/isc/timer.c +++ b/lib/isc/timer.c @@ -727,7 +727,6 @@ isc_timermgr_poke(isc_timermgr_t *manager0) { void isc_timermgr_destroy(isc_timermgr_t **managerp) { isc__timermgr_t *manager; - isc_mem_t *mctx; /* * Destroy a timer manager. @@ -762,9 +761,7 @@ isc_timermgr_destroy(isc_timermgr_t **managerp) { isc_heap_destroy(&manager->heap); manager->common.impmagic = 0; manager->common.magic = 0; - mctx = manager->mctx; - isc_mem_put(mctx, manager, sizeof(*manager)); - isc_mem_detach(&mctx); + isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager)); *managerp = NULL; diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c index b10868c525..b303134efc 100644 --- a/lib/isc/unix/socket.c +++ b/lib/isc/unix/socket.c @@ -3851,8 +3851,6 @@ isc_socketmgr_setstats(isc_socketmgr_t *manager0, isc_stats_t *stats) { void isc_socketmgr_destroy(isc_socketmgr_t **managerp) { isc__socketmgr_t *manager; - isc_mem_t *mctx; - int i; /* * Destroy a socket manager. @@ -3879,14 +3877,14 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) { * half of the pipe, which will send EOF to the read half. * This is currently a no-op in the non-threaded case. */ - for (i = 0; i < manager->nthreads; i++) { + for (int i = 0; i < manager->nthreads; i++) { select_poke(manager, i, 0, SELECT_POKE_SHUTDOWN); } /* * Wait for thread to exit. */ - for (i = 0; i < manager->nthreads; i++) { + for (int i = 0; i < manager->nthreads; i++) { isc_result_t result; result = isc_thread_join(manager->threads[i].thread, NULL); if (result != ISC_R_SUCCESS) { @@ -3908,10 +3906,7 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) { isc_mutex_destroy(&manager->lock); manager->common.magic = 0; manager->common.impmagic = 0; - mctx= manager->mctx; - isc_mem_put(mctx, manager, sizeof(*manager)); - - isc_mem_detach(&mctx); + isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager)); *managerp = NULL; diff --git a/lib/isc/win32/socket.c b/lib/isc/win32/socket.c index 36d5c78a76..54b76a7204 100644 --- a/lib/isc/win32/socket.c +++ b/lib/isc/win32/socket.c @@ -2537,8 +2537,6 @@ isc_socketmgr_setstats(isc_socketmgr_t *manager, isc_stats_t *stats) { void isc_socketmgr_destroy(isc_socketmgr_t **managerp) { isc_socketmgr_t *manager; - int i; - isc_mem_t *mctx; /* * Destroy a socket manager. @@ -2570,7 +2568,7 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) { /* * Wait for threads to exit. */ - for (i = 0; i < manager->maxIOCPThreads; i++) { + for (int i = 0; i < manager->maxIOCPThreads; i++) { if (isc_thread_join((isc_thread_t) manager->hIOCPThreads[i], NULL) != ISC_R_SUCCESS) UNEXPECTED_ERROR(__FILE__, __LINE__, @@ -2585,13 +2583,11 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) { (void)isc_condition_destroy(&manager->shutdown_ok); isc_mutex_destroy(&manager->lock); - if (manager->stats != NULL) + if (manager->stats != NULL) { isc_stats_detach(&manager->stats); + } manager->magic = 0; - mctx= manager->mctx; - isc_mem_put(mctx, manager, sizeof(*manager)); - - isc_mem_detach(&mctx); + isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager)); *managerp = NULL; }