[asterisk-bugs] [JIRA] (ASTERISK-28951) Inconsistent behaviour queues.conf when there is (not) a [general] section

Walter Doekes (JIRA) noreply at issues.asterisk.org
Mon Jun 15 04:58:25 CDT 2020


Walter Doekes created ASTERISK-28951:
----------------------------------------

             Summary: Inconsistent behaviour queues.conf when there is (not) a [general] section
                 Key: ASTERISK-28951
                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-28951
             Project: Asterisk
          Issue Type: Bug
      Security Level: None
          Components: Applications/app_queue
    Affects Versions: 13.34.0
            Reporter: Walter Doekes
            Severity: Trivial


app_queue suggests these "defaults" at the top of the file:
{code}
/*! \brief queues.conf [general] option */
static int autofill_default = 1;

/*! \brief queues.conf [general] option */
static int montype_default = 0;

/*! \brief queues.conf [general] option */
static int shared_lastcall = 1;
{code}
The queue reload code contains this:
{code}
/*! Set the global queue parameters as defined in the "general" section of queues.conf */
static void queue_set_global_params(struct ast_config *cfg)
{
...
        autofill_default = 0;
        if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
                autofill_default = ast_true(general_val);
...
        shared_lastcall = 0;
        if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
                shared_lastcall = ast_true(general_val);
{code}
And that code is ran here:
{code}
static int reload_queues(int reload, struct ast_flags *mask, const char *queuename)
...
        if (!(cfg = ast_config_load("queues.conf", config_flags))) {
...
        while ((cat = ast_category_browse(cfg, cat)) ) {
                if (!strcasecmp(cat, "general") && queue_reload) {
                        queue_set_global_params(cfg);
                        continue;
{code}
That means:
- if there is a queues.conf
- it has a {{\[general]}} section (or _had_ a such a section)
- the autofill_default default is 0, and the shared_lastcall default is 0

If there is NO {{\[general]}} section:
- the autofill_default default is 1, and the shared_lastcall default is 1

That's confusing, and also, it disagrees with the sample cfg:
{noformat}
;    in most cases, you will want to enable this behavior. If you
;    do not specify or comment out this option, it will default to yes.
;
;autofill = no
...
; The default value is no.
;
;shared_lastcall=no
{noformat}

That last one was changed here:
{noformat}
commit 4b50e3f1ee84ae29da6d9eb3cfd9896a49d2394b
Author: Mark Michelson <mmichelson at digium.com>
Date:   Mon Aug 27 17:52:16 2012 +0000

    Fix incorrectly documented option in queues.conf
    
    sharedlastcall defaults to "no" not "yes"
{noformat}
But the autofill_default was not. And they still differ depending on the existence of the general section.

Suggested fix:
{noformat}
diff --git a/apps/app_queue.c b/apps/app_queue.c
index 3a572efc2d..1e786fc39f 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -1363,34 +1363,34 @@ static char *app_ql = "QueueLog" ;
 static const char * const pm_family = "Queue/PersistentMembers";
 
 /*! \brief queues.conf [general] option */
-static int queue_persistent_members = 0;
+static int queue_persistent_members;
 
-/*! \brief queues.conf per-queue weight option */
-static int use_weight = 0;
+/*! \brief Records that one or more queues use weight */
+static int use_weight;
 
 /*! \brief queues.conf [general] option */
-static int autofill_default = 1;
+static int autofill_default;
 
 /*! \brief queues.conf [general] option */
-static int montype_default = 0;
+static int montype_default;
 
 /*! \brief queues.conf [general] option */
-static int shared_lastcall = 1;
+static int shared_lastcall;
 
 /*! \brief queuesrules.conf [general] option */
-static int realtime_rules = 0;
+static int realtime_rules;
 
 /*! \brief Subscription to device state change messages */
 static struct stasis_subscription *device_state_sub;
 
 /*! \brief queues.conf [general] option */
-static int update_cdr = 0;
+static int update_cdr;
 
 /*! \brief queues.conf [general] option */
-static int negative_penalty_invalid = 0;
+static int negative_penalty_invalid;
 
 /*! \brief queues.conf [general] option */
-static int log_membername_as_agent = 0;
+static int log_membername_as_agent;
 
 /*! \brief name of the ringinuse field in the realtime database */
 static char *realtime_ringinuse_field;
@@ -8722,14 +8722,19 @@ static struct ast_custom_function queuememberpenalty_function = {
 	.write = queue_function_memberpenalty_write,
 };
 
+/*! Reset the global queue rules parameters even if there is no "general" section of queuerules.conf */
+static void queue_rules_reset_global_params()
+{
+	realtime_rules = 0;
+}
+
 /*! Set the global queue rules parameters as defined in the "general" section of queuerules.conf */
 static void queue_rules_set_global_params(struct ast_config *cfg)
 {
-        const char *general_val = NULL;
-        realtime_rules = 0;
-        if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
-                realtime_rules = ast_true(general_val);
-        }
+	const char *general_val = NULL;
+	if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
+		realtime_rules = ast_true(general_val);
+	}
 }
 
 /*! \brief Reload the rules defined in queuerules.conf
@@ -8764,6 +8769,7 @@ static int reload_queue_rules(int reload)
 			ast_free(pr_iter);
 		ast_free(rl_iter);
 	}
+	queue_rules_reset_global_params();
 	while ((rulecat = ast_category_browse(cfg, rulecat))) {
 		if (!strcasecmp(rulecat, "general")) {
 			queue_rules_set_global_params(cfg);
@@ -8795,36 +8801,41 @@ static int reload_queue_rules(int reload)
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
+/*! Always set the global queue defaults, even if there is no "general" section in queues.conf */
+static void queue_reset_global_params()
+{
+	queue_persistent_members = 0;
+	autofill_default = 0;
+	montype_default = 0;
+	update_cdr = 0;
+	shared_lastcall = 0;
+	negative_penalty_invalid = 0;
+	log_membername_as_agent = 0;
+}
+
 /*! Set the global queue parameters as defined in the "general" section of queues.conf */
 static void queue_set_global_params(struct ast_config *cfg)
 {
 	const char *general_val = NULL;
-	queue_persistent_members = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "persistentmembers"))) {
 		queue_persistent_members = ast_true(general_val);
 	}
-	autofill_default = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
 		autofill_default = ast_true(general_val);
 	}
-	montype_default = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "monitor-type"))) {
 		if (!strcasecmp(general_val, "mixmonitor"))
 			montype_default = 1;
 	}
-	update_cdr = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "updatecdr"))) {
 		update_cdr = ast_true(general_val);
 	}
-	shared_lastcall = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
 		shared_lastcall = ast_true(general_val);
 	}
-	negative_penalty_invalid = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "negative_penalty_invalid"))) {
 		negative_penalty_invalid = ast_true(general_val);
 	}
-	log_membername_as_agent = 0;
 	if ((general_val = ast_variable_retrieve(cfg, "general", "log_membername_as_agent"))) {
 		log_membername_as_agent = ast_true(general_val);
 	}
@@ -9128,6 +9139,7 @@ static int reload_queues(int reload, struct ast_flags *mask, const char *queuena
 
 	/* Chug through config file. */
 	cat = NULL;
+	queue_reset_global_params();
 	while ((cat = ast_category_browse(cfg, cat)) ) {
 		if (!strcasecmp(cat, "general") && queue_reload) {
 			queue_set_global_params(cfg);
diff --git a/configs/samples/queues.conf.sample b/configs/samples/queues.conf.sample
index 5c8a80ffee..f1694929fb 100644
--- a/configs/samples/queues.conf.sample
+++ b/configs/samples/queues.conf.sample
@@ -23,7 +23,7 @@ persistentmembers = yes
 ;    no more available members or no more waiting callers. This is
 ;    probably more along the lines of how a queue should work and
 ;    in most cases, you will want to enable this behavior. If you
-;    do not specify or comment out this option, it will default to yes.
+;    do not specify or comment out this option, it will default to no.
 ;
 ;autofill = no
 ;
{noformat}
Unfortunately, this change will change things for the current users who don't have a {{\[general]}} section.

So, suggested target for this change?
- master only?
- 13 and up, adding an appropriate notice in CHANGES?



--
This message was sent by Atlassian JIRA
(v6.2#6252)



More information about the asterisk-bugs mailing list