- 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 7add3ba092ce92b27fa525b663568c3fe6728022..f6767e17b45ee97aae8df04974c6e2a6ace3b1dd 100644 (file)
--- 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 c7a3f4f529522db09fb8346182442d729de72bdb..a500c75e786ab1ec17709aefebdb74ca1f6e8056 100644 (file)
--- 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 4fb8b005c1971201ab36a0e6b28be9aa2b4bf2e0..8f5a291af6781362d86413c006850b8c0f6f5539 100644 (file)
--- 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 af20c4261b7dbcb39ad335353239ef724bd998a6..e3d3099aa3beedfc70bc59d49e075b82996666f0 100644 (file)
--- 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;