From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 4 Feb 2021 19:34:59 +0000 (+0000)
Subject: Move into a loop, so it's clearer what's going on
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=530b9cd86f5fc9bcaa649c5faec2f0f230970ddd;p=thirdparty%2Ffreeradius-server.git

Move into a loop, so it's clearer what's going on
---

diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c
index 5aa31beb395..855c4c7f622 100644
--- a/src/lib/io/worker.c
+++ b/src/lib/io/worker.c
@@ -877,108 +877,97 @@ static void worker_run_request(fr_worker_t *worker, fr_time_t start)
 
 	now = start;
 
-redo:
-	request = fr_heap_pop(worker->runnable);
-	if (!request) return;
-
-	REQUEST_VERIFY(request);
-	fr_assert(request->runnable_id < 0);
-	fr_time_tracking_resume(&request->async->tracking, now);
+	while ((request = fr_heap_pop(worker->runnable))) {
+		REQUEST_VERIFY(request);
+		fr_assert(request->runnable_id < 0);
+		fr_time_tracking_resume(&request->async->tracking, now);
 
-	fr_assert(request->parent == NULL);
-	fr_assert(request->async->process != NULL);
-	fr_assert(request->async->fake || (request->async->listen != NULL));
-	fr_assert(request->runnable_id < 0); /* removed from the runnable heap */
+		fr_assert(request->parent == NULL);
+		fr_assert(request->async->process != NULL);
+		fr_assert(request->async->fake || (request->async->listen != NULL));
 
-	RDEBUG("Running request");
+		RDEBUG("Running request");
 
-	/*
-	 *	For real requests, if the channel is gone, just stop
-	 *	the request and free it.
-	 */
-	if (!request->async->fake && !fr_channel_active(request->async->channel)) {
-		worker_stop_request(worker, request, now);
-		talloc_free(request);
-		return;
-	}
-
-	/*
-	 *	Everything else, run the request.
-	 */
-	request->async->process(&final, &(module_ctx_t){ .instance = request->async->process_inst }, request);
+		/*
+		 *	For real requests, if the channel is gone, just stop
+		 *	the request and free it.
+		 */
+		if (!request->async->fake && !fr_channel_active(request->async->channel)) {
+			worker_stop_request(worker, request, now);
+			talloc_free(request);
+			return;
+		}
 
-	/*
-	 *	Figure out what to do next.
-	 */
-	switch (final) {
-	case RLM_MODULE_HANDLED:
 		/*
-		 *	Done: don't send a reply.
+		 *	Everything else, run the request.
 		 */
-		break;
+		request->async->process(&final, &(module_ctx_t){ .instance = request->async->process_inst }, request);
 
-	case RLM_MODULE_FAIL:
-	default:
 		/*
-		 *	Something went wrong.  It's done, but we don't send a reply.
+		 *	Figure out what to do next.
 		 */
-		break;
+		switch (final) {
+		case RLM_MODULE_HANDLED:
+			/*
+			 *	Done: don't send a reply.
+			 */
+			break;
 
-	case RLM_MODULE_YIELD:
-		now = fr_time();
-		fr_time_tracking_yield(&request->async->tracking, now);
-		goto keep_going;
+		case RLM_MODULE_FAIL:
+		default:
+			/*
+			 *	Something went wrong.  It's done, but we don't send a reply.
+			 */
+			break;
+
+		case RLM_MODULE_YIELD:
+			now = fr_time();
+			fr_time_tracking_yield(&request->async->tracking, now);
+			goto keep_going;
+
+		case RLM_MODULE_OK:
+			break;
+		}
 
-	case RLM_MODULE_OK:
 		/*
-		 *	Don't reply to internally generated request.
+		 *	If we're running a real request, then the final
+		 *	indentation MUST be zero.  Otherwise we skipped
+		 *	something!
+		 *
+		 *	Also check that the request is NOT marked as
+		 *	"yielded", but is in fact done.
+		 *
+		 *	@todo - check that the stack is at frame 0, otherwise
+		 *	more things have gone wrong.
 		 */
-		if (request->parent || request->async->fake) break;
-		break;
-	}
+		fr_assert_msg(request->parent || (request->log.unlang_indent == 0),
+			      "Request %s bad log indentation - expected 0 got %u", request->name, request->log.unlang_indent);
+		fr_assert_msg(!unlang_interpret_is_resumable(request),
+			      "Request %s is marked as yielded at end of processing", request->name);
 
-	/*
-	 *	If we're running a real request, then the final
-	 *	indentation MUST be zero.  Otherwise we skipped
-	 *	something!
-	 *
-	 *	Also check that the request is NOT marked as
-	 *	"yielded", but is in fact done.
-	 *
-	 *	@todo - check that the stack is at frame 0, otherwise
-	 *	more things have gone wrong.
-	 */
-	fr_assert_msg(request->parent || (request->log.unlang_indent == 0),
-		      "Request %s bad log indentation - expected 0 got %u", request->name, request->log.unlang_indent);
-	fr_assert_msg(!unlang_interpret_is_resumable(request),
-		      "Request %s is marked as yielded at end of processing", request->name);
-
-	RDEBUG("Done request");
+		RDEBUG("Done request");
 
-	/*
-	 *	Only real packets are in the dedup tree.  And even
-	 *	then, only some of the time.
-	 */
-	if (!request->async->fake && request->async->listen->track_duplicates) {
-		(void) rbtree_deletebydata(worker->dedup, request);
-	}
+		/*
+		 *	Only real packets are in the dedup tree.  And even
+		 *	then, only some of the time.
+		 */
+		if (!request->async->fake && request->async->listen->track_duplicates) {
+			(void) rbtree_deletebydata(worker->dedup, request);
+		}
 
-	now = fr_time();
-	worker_send_reply(worker, request, 0, now);
-	now = fr_time();
+		now = fr_time();
+		worker_send_reply(worker, request, 0, now);
+		now = fr_time();
 
-keep_going:
-	/*
-	 *	If this request ran for less than 0.1ms, then go run
-	 *	another request.  This change means that the worker
-	 *	checks the event loop only 10K times per second, which
-	 *	should be good enough.
-	 */
-	if ((now - start) > (NSEC / 100000)) {
-		return;
+	keep_going:
+		/*
+		 *	If this request ran for less than 0.1ms, then go run
+		 *	another request.  This change means that the worker
+		 *	checks the event loop only 10K times per second, which
+		 *	should be good enough.
+		 */
+		if ((now - start) > (NSEC / 100000)) break;
 	}
-
-	goto redo;
 }