[6.0] dcbc80c6b reshuffle HSH_Lookup() code a bit to condense and clarify

[Dridi Boukelmoune]

commit dcbc80c6b3669858c0eed90044b8d96c59587ae3
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Tue Jun 19 17:02:36 2018 +0200

    reshuffle HSH_Lookup() code a bit to condense and clarify
    
    With the req.grace reintroduction, we got two identical cases where
    we insert a busy object: When there is no exp_oc (obviously) or
    when the exp_oc does not fulfil the req.grace limit. This lead
    to some code duplication.
    
    IMHO, handling first the case where we need to insert a busy object
    (with or without exp_oc) and then handling the exception that we
    do not wait on a busy object if the exp_oc is in grace is cleaner.
    
    Please note that comparing this change using diff -u does not
    make much sense, I believe the simplicity of the reshuffling
    change becomes clear when looking at the old and new code side-by-side

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 38531b149..3fbbc702d 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -478,58 +478,45 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 		busy_found = 0;
 	}
 
-	if (exp_oc != NULL && EXP_Ttl_grace(req, exp_oc) < req->t_req) {
-		/* the newest object is out of grace */
-		if (!busy_found) {
-			/*
-			 * here we get to insert the busy object. This
-			 * translates to a MISS with a 304 candidate.
-			 */
+	if (!busy_found) {
+		/* Insert objcore in objecthead */
+		*bocp = hsh_insert_busyobj(wrk, oh);
+
+		if (exp_oc != NULL) {
 			assert(oh->refcnt > 1);
 			assert(exp_oc->objhead == oh);
 			exp_oc->refcnt++;
-			/* Insert objcore in objecthead and release mutex */
-			*bocp = hsh_insert_busyobj(wrk, oh);
-			/* NB: no deref of objhead, new object inherits reference */
 			Lck_Unlock(&oh->mtx);
 			*ocp = exp_oc;
-			return (HSH_MISS);
+
+			if (EXP_Ttl_grace(req, exp_oc) < req->t_req) {
+				retval = HSH_MISS;
+			} else {
+				if (exp_oc->hits < LONG_MAX)
+					exp_oc->hits++;
+				retval = HSH_EXPBUSY;
+			}
 		} else {
-			/* we have no use for this very expired object */
-			exp_oc = NULL;
+			Lck_Unlock(&oh->mtx);
+			retval = HSH_MISS;
 		}
+
+		return (retval);
 	}
-	if (exp_oc != NULL) {
-		/*
-		 * here the object is within grace, so we expect it to
-		 * be delivered
-		 */
+
+	AN(busy_found);
+	if (exp_oc != NULL && EXP_Ttl_grace(req, exp_oc) >= req->t_req) {
+		/* we do not wait on the busy object if in grace */
 		assert(oh->refcnt > 1);
 		assert(exp_oc->objhead == oh);
 		exp_oc->refcnt++;
-
-		if (!busy_found) {
-			*bocp = hsh_insert_busyobj(wrk, oh);
-			retval = HSH_EXPBUSY;
-		} else {
-			AZ(req->hash_ignore_busy);
-			retval = HSH_EXP;
-		}
-		if (exp_oc->hits < LONG_MAX)
-			exp_oc->hits++;
 		Lck_Unlock(&oh->mtx);
-		if (retval == HSH_EXP)
-			assert(HSH_DerefObjHead(wrk, &oh));
 		*ocp = exp_oc;
-		return (retval);
-	}
 
-	if (!busy_found) {
-		/* Insert objcore in objecthead and release mutex */
-		*bocp = hsh_insert_busyobj(wrk, oh);
-		/* NB: no deref of objhead, new object inherits reference */
-		Lck_Unlock(&oh->mtx);
-		return (HSH_MISS);
+		assert(HSH_DerefObjHead(wrk, &oh));
+		if (exp_oc->hits < LONG_MAX)
+			exp_oc->hits++;
+		return (HSH_EXP);
 	}
 
 	/* There are one or more busy objects, wait for them */