[Asterisk-code-review] app_queue: Fix a few member pause bugs (...asterisk[16])

[Sean Bright]

Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/11170


Change subject: app_queue: Fix a few member pause bugs
......................................................................

app_queue: Fix a few member pause bugs

* When setting member->paused, make sure to always set member->lastpaused
  and member->reason_paused if applicable. Created a simple wrapper
  function to handle this.

* When printing the time the member was paused, use member->lastpaused,
  regardless of a reason phrase being present or not.

ASTERISK-27541 #close
Reported by: César Benjamín García Martínez

Change-Id: I80507e05e3c9bc61843104c405a9ed9c0de1594e
---
M apps/app_queue.c
1 file changed, 22 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/70/11170/1

diff --git a/apps/app_queue.c b/apps/app_queue.c
index 132621d..b2114a4 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -2683,6 +2683,19 @@
 	}
 }
 
+/*! \brief Wrapper to ensure that the last pause time is set */
+static void member_set_paused(struct member *mem, int paused, const char *reason)
+{
+	mem->reason_paused[0] = '\0';
+	mem->paused = paused;
+	if (paused) {
+		time(&mem->lastpause);
+		if (!ast_strlen_zero(reason)) {
+			ast_copy_string(mem->reason_paused, reason, sizeof(mem->reason_paused));
+		}
+	}
+}
+
 /*! \brief allocate space for new queue member and set fields based on parameters passed */
 static struct member *create_queue_member(const char *interface, const char *membername, int penalty, int paused, const char *state_interface, int ringinuse, int wrapuptime)
 {
@@ -2691,11 +2704,8 @@
 	if ((cur = ao2_alloc(sizeof(*cur), destroy_queue_member_cb))) {
 		cur->ringinuse = ringinuse;
 		cur->penalty = penalty;
-		cur->paused = paused;
 		cur->wrapuptime = wrapuptime;
-		if (paused) {
-			time(&cur->lastpause); /* Update time of last pause */
-		}
+		member_set_paused(cur, paused, NULL);
 		ast_copy_string(cur->interface, interface, sizeof(cur->interface));
 		if (!ast_strlen_zero(state_interface)) {
 			ast_copy_string(cur->state_interface, state_interface, sizeof(cur->state_interface));
@@ -3461,7 +3471,7 @@
 			m->dead = 0;	/* Do not delete this one. */
 			ast_copy_string(m->rt_uniqueid, rt_uniqueid, sizeof(m->rt_uniqueid));
 			if (paused_str) {
-				m->paused = paused;
+				member_set_paused(m, paused, NULL);
 				ast_devstate_changed(m->paused ? QUEUE_PAUSED_DEVSTATE : QUEUE_UNPAUSED_DEVSTATE,
 					AST_DEVSTATE_CACHABLE, "Queue:%s_pause_%s", q->name, m->interface);
 			}
@@ -7488,15 +7498,7 @@
 		}
 	}
 
-	mem->paused = paused;
-	if (paused) {
-		time(&mem->lastpause); /* update last pause field */
-	}
-	if (paused && !ast_strlen_zero(reason)) {
-		ast_copy_string(mem->reason_paused, reason, sizeof(mem->reason_paused));
-	} else {
-		mem->reason_paused[0] = '\0';
-	}
+	member_set_paused(mem, paused, mem->reason_paused);
 
 	ast_devstate_changed(mem->paused ? QUEUE_PAUSED_DEVSTATE : QUEUE_UNPAUSED_DEVSTATE,
 		AST_DEVSTATE_CACHABLE, "Queue:%s_pause_%s", q->name, mem->interface);
@@ -9749,13 +9751,12 @@
 					mem->realtime ? ast_term_color(COLOR_MAGENTA, COLOR_BLACK) : "", mem->realtime ? " (realtime)" : "", ast_term_reset(),
 					mem->starttime ? ast_term_color(COLOR_BROWN, COLOR_BLACK) : "", mem->starttime ? " (in call)" : "", ast_term_reset());
 				if (mem->paused) {
-					if (ast_strlen_zero(mem->reason_paused)) {
-						ast_str_append(&out, 0, " %s(paused was %ld secs ago)%s",
-							ast_term_color(COLOR_BROWN, COLOR_BLACK), (long) (time(NULL) - mem->lastpause), ast_term_reset());
-					} else {
-						ast_str_append(&out, 0, " %s(paused:%s was %ld secs ago)%s", ast_term_color(COLOR_BROWN, COLOR_BLACK),
-							mem->reason_paused,  (long) (time(NULL) - mem->lastcall), ast_term_reset());
-					}
+					ast_str_append(&out, 0, " %s(paused%s%s was %ld secs ago)%s",
+						ast_term_color(COLOR_BROWN, COLOR_BLACK),
+						ast_strlen_zero(mem->reason_paused) ? "" : ":",
+						ast_strlen_zero(mem->reason_paused) ? "" : mem->reason_paused,
+						(long) (time(NULL) - mem->lastpause),
+						ast_term_reset());
 				}
 
 				ast_str_append(&out, 0, " (%s%s%s)",

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11170
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I80507e05e3c9bc61843104c405a9ed9c0de1594e
Gerrit-Change-Number: 11170
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190325/f203e2e6/attachment-0001.html>