From: Wouter Wijngaards Date: Thu, 28 Jun 2012 14:18:41 +0000 (+0000) Subject: - code review: return value of cache_store can be ignored for better X-Git-Tag: release-1.4.18rc1~19 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1467c5de5246b240fb8a92fab7ab99085f77d146;p=thirdparty%2Funbound.git - code review: return value of cache_store can be ignored for better performance in out of memory conditions. git-svn-id: file:///svn/unbound/trunk@2704 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/doc/Changelog b/doc/Changelog index 7add3ba09..f6767e17b 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,5 +1,7 @@ 28 June 2012: Wouter - detect if openssl has FIPS_mode. + - code review: return value of cache_store can be ignored for better + performance in out of memory conditions. 25 June 2012: Wouter - disable RSAMD5 if in FIPS mode (for openssl and for libnss). diff --git a/iterator/iter_utils.c b/iterator/iter_utils.c index c7a3f4f52..a500c75e7 100644 --- a/iterator/iter_utils.c +++ b/iterator/iter_utils.c @@ -418,13 +418,14 @@ dns_copy_msg(struct dns_msg* from, struct regional* region) return m; } -int +void iter_dns_store(struct module_env* env, struct query_info* msgqinf, struct reply_info* msgrep, int is_referral, uint32_t leeway, int pside, struct regional* region) { - return dns_cache_store(env, msgqinf, msgrep, is_referral, leeway, - pside, region); + if(!dns_cache_store(env, msgqinf, msgrep, is_referral, leeway, + pside, region)) + log_err("out of memory: cannot store data in cache"); } int diff --git a/iterator/iter_utils.h b/iterator/iter_utils.h index 4fb8b005c..8f5a291af 100644 --- a/iterator/iter_utils.h +++ b/iterator/iter_utils.h @@ -124,9 +124,13 @@ struct dns_msg* dns_copy_msg(struct dns_msg* from, struct regional* regional); * @param pside: true if dp is parentside, thus message is 'fresh' and NS * can be prefetch-updates. * @param region: to copy modified (cache is better) rrs back to. - * @return 0 on alloc error (out of memory). + * @return void, because we are not interested in alloc errors, + * the iterator and validator can operate on the results in their + * scratch space (the qstate.region) and are not dependent on the cache. + * It is useful to log the alloc failure (for the server operator), + * but the query resolution can continue without cache storage. */ -int iter_dns_store(struct module_env* env, struct query_info* qinf, +void iter_dns_store(struct module_env* env, struct query_info* qinf, struct reply_info* rep, int is_referral, uint32_t leeway, int pside, struct regional* region); diff --git a/iterator/iterator.c b/iterator/iterator.c index af20c4261..e3d3099aa 100644 --- a/iterator/iterator.c +++ b/iterator/iterator.c @@ -259,9 +259,7 @@ error_response_cache(struct module_qstate* qstate, int id, int rcode) /* do not waste time trying to validate this servfail */ err.security = sec_status_indeterminate; verbose(VERB_ALGO, "store error response in message cache"); - if(!iter_dns_store(qstate->env, &qstate->qinfo, &err, 0, 0, 0, NULL)) { - log_err("error_response_cache: could not store error (nomem)"); - } + iter_dns_store(qstate->env, &qstate->qinfo, &err, 0, 0, 0, NULL); return error_response(qstate, id, rcode); } @@ -1908,11 +1906,10 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq, && iter_ds_toolow(iq->response, iq->dp) && iter_dp_cangodown(&iq->qchase, iq->dp)) return processDSNSFind(qstate, iq, id); - if(!iter_dns_store(qstate->env, &iq->response->qinfo, + iter_dns_store(qstate->env, &iq->response->qinfo, iq->response->rep, 0, qstate->prefetch_leeway, iq->dp&&iq->dp->has_parent_side_NS, - qstate->region)) - return error_response(qstate, id, LDNS_RCODE_SERVFAIL); + qstate->region); /* close down outstanding requests to be discarded */ outbound_list_clear(&iq->outlist); iq->num_current_queries = 0; @@ -1949,10 +1946,8 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq, )) { /* Store the referral under the current query */ /* no prefetch-leeway, since its not the answer */ - if(!iter_dns_store(qstate->env, &iq->response->qinfo, - iq->response->rep, 1, 0, 0, NULL)) - return error_response(qstate, id, - LDNS_RCODE_SERVFAIL); + iter_dns_store(qstate->env, &iq->response->qinfo, + iq->response->rep, 1, 0, 0, NULL); if(iq->store_parent_NS) iter_store_parentside_NS(qstate->env, iq->response->rep); @@ -2042,10 +2037,9 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq, /* NOTE : set referral=1, so that rrsets get stored but not * the partial query answer (CNAME only). */ /* prefetchleeway applied because this updates answer parts */ - if(!iter_dns_store(qstate->env, &iq->response->qinfo, + iter_dns_store(qstate->env, &iq->response->qinfo, iq->response->rep, 1, qstate->prefetch_leeway, - iq->dp&&iq->dp->has_parent_side_NS, NULL)) - return error_response(qstate, id, LDNS_RCODE_SERVFAIL); + iq->dp&&iq->dp->has_parent_side_NS, NULL); /* set the current request's qname to the new value. */ iq->qchase.qname = sname; iq->qchase.qname_len = snamelen; @@ -2555,12 +2549,10 @@ processFinished(struct module_qstate* qstate, struct iter_qstate* iq, * but only if we did recursion. The nonrecursion referral * from cache does not need to be stored in the msg cache. */ if(qstate->query_flags&BIT_RD) { - if(!iter_dns_store(qstate->env, &qstate->qinfo, + iter_dns_store(qstate->env, &qstate->qinfo, iq->response->rep, 0, qstate->prefetch_leeway, iq->dp&&iq->dp->has_parent_side_NS, - qstate->region)) - return error_response(qstate, id, - LDNS_RCODE_SERVFAIL); + qstate->region); } } qstate->return_rcode = LDNS_RCODE_NOERROR;