main/cdrs: Preserve context/extension when executing a Macro or GoSub

The context/extension in a CDR is generally considered the destination of a
call. When looking at a 2-party call CDR, users will typically be presented
with the following:

context    exten      channel     dest_channel app  data
default    1000       SIP/8675309 SIP/1000     Dial SIP/1000,,20

However, if the Dial actually takes place in a Macro, the current behaviour
in 12 will result in the following CDR:

context    exten      channel     dest_channel app  data
macro-dial s          SIP/8675309 SIP/1000     Dial SIP/1000,,20

The same is true of a GoSub:

context    exten      channel     dest_channel app  data
subs       dial_stuff SIP/8675309 SIP/1000     Dial SIP/1000,,20

This generally makes the context/exten fields less than useful.

It isn't hard to preserve these values in the CDR state machine; however, we
need to have something that informs us when a channel is executing a
subroutine. Prior to this patch, there isn't anything that does this.

This patch solves this problem by adding a new channel flag,
AST_FLAG_SUBROUTINE_EXEC. This flag is set on a channel when it executes a
Macro or a GoSub. The CDR engine looks for this value when updating a Party A
snapshot; if the flag is present, we don't override the context/exten on the
main CDR object. In a funny quirk, executing a hangup handler must *not* abide
by this logic, as the endbeforehexten logic assumes that the user wants to see
data that occurs in hangup logic, which includes those subroutines. Since
those execute outside of a typical Dial operation (and will typically have
their own dedicated CDR anyway), this is unlikely to cause any heartburn.

Review: https://reviewboard.asterisk.org/r/3962/

ASTERISK-24254 #close
Reported by: tm1000, Tony Lewis
Tested by: Tony Lewis

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@422718 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/apps/app_macro.c b/apps/app_macro.c
index 9ec3b8636ff3217f38aa224a93dc738a0913f749..874895467842f6ca9cab9252a34572fecf8694da 100644 (file)
--- a/apps/app_macro.c
+++ b/apps/app_macro.c
@@ -248,6 +248,7 @@ static int _macro_exec(struct ast_channel *chan, const char *data, int exclusive
        char *save_macro_context;
        char *save_macro_priority;
        char *save_macro_offset;
+       int save_in_subroutine;
        struct ast_datastore *macro_store = ast_channel_datastore_find(chan, &macro_ds_info, NULL);
 
        if (ast_strlen_zero(data)) {
@@ -329,6 +330,7 @@ static int _macro_exec(struct ast_channel *chan, const char *data, int exclusive
        }
 
        /* Save old info */
+       ast_channel_lock(chan);
        oldpriority = ast_channel_priority(chan);
        ast_copy_string(oldexten, ast_channel_exten(chan), sizeof(oldexten));
        ast_copy_string(oldcontext, ast_channel_context(chan), sizeof(oldcontext));
@@ -355,12 +357,14 @@ static int _macro_exec(struct ast_channel *chan, const char *data, int exclusive
 
        pbx_builtin_setvar_helper(chan, "MACRO_DEPTH", depthc);
 
+       save_in_subroutine = ast_test_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
+       ast_set_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
+
        /* Setup environment for new run */
        ast_channel_exten_set(chan, "s");
        ast_channel_context_set(chan, fullmacro);
        ast_channel_priority_set(chan, 1);
 
-       ast_channel_lock(chan);
        while((cur = strsep(&rest, ",")) && (argc < MAX_ARGS)) {
                const char *argp;
                /* Save copy of old arguments if we're overwriting some, otherwise
@@ -513,6 +517,7 @@ static int _macro_exec(struct ast_channel *chan, const char *data, int exclusive
        snprintf(depthc, sizeof(depthc), "%d", depth);
        pbx_builtin_setvar_helper(chan, "MACRO_DEPTH", depthc);
        ast_set2_flag(ast_channel_flags(chan), autoloopflag, AST_FLAG_IN_AUTOLOOP);
+       ast_set2_flag(ast_channel_flags(chan), save_in_subroutine, AST_FLAG_SUBROUTINE_EXEC);
 
        for (x = 1; x < argc; x++) {
                /* Restore old arguments and delete ours */

diff --git a/apps/app_stack.c b/apps/app_stack.c
index 0f6f7a6bf37b0e69384d5db48bf09e9ab51ea0a4..e21cafc8eb8e0fbe8fda624b9a3496a779e4ce66 100644 (file)
--- a/apps/app_stack.c
+++ b/apps/app_stack.c
@@ -251,6 +251,8 @@ struct gosub_stack_frame {
        int priority;
        /*! TRUE if the return location marks the end of a special routine. */
        unsigned int is_special:1;
+       /*! Whether or not we were in a subroutine when this one was created */
+       unsigned int in_subroutine:1;
        char *context;
        char extension[0];
 };
@@ -310,7 +312,7 @@ static void gosub_release_frame(struct ast_channel *chan, struct gosub_stack_fra
        ast_free(frame);
 }
 
-static struct gosub_stack_frame *gosub_allocate_frame(const char *context, const char *extension, int priority, unsigned char arguments)
+static struct gosub_stack_frame *gosub_allocate_frame(const char *context, const char *extension, int priority, int in_subroutine, unsigned char arguments)
 {
        struct gosub_stack_frame *new = NULL;
        int len_extension = strlen(extension), len_context = strlen(context);
@@ -321,6 +323,7 @@ static struct gosub_stack_frame *gosub_allocate_frame(const char *context, const
                new->context = new->extension + len_extension + 1;
                strcpy(new->context, context);
                new->priority = priority;
+               new->in_subroutine = in_subroutine ? 1 : 0;
                new->arguments = arguments;
        }
        return new;
@@ -416,6 +419,7 @@ static int return_exec(struct ast_channel *chan, const char *data)
                --oldframe->priority;
        }
        ast_channel_priority_set(chan, oldframe->priority);
+       ast_set2_flag(ast_channel_flags(chan), oldframe->in_subroutine, AST_FLAG_SUBROUTINE_EXEC);
 
        gosub_release_frame(chan, oldframe);
 
@@ -524,6 +528,7 @@ static int gosub_exec(struct ast_channel *chan, const char *data)
        char *orig_exten;
        char *dest_context;
        char *dest_exten;
+       int orig_in_subroutine;
        int orig_priority;
        int dest_priority;
        int i;
@@ -563,6 +568,7 @@ static int gosub_exec(struct ast_channel *chan, const char *data)
        orig_context = ast_strdupa(ast_channel_context(chan));
        orig_exten = ast_strdupa(ast_channel_exten(chan));
        orig_priority = ast_channel_priority(chan);
+       orig_in_subroutine = ast_test_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
        ast_channel_unlock(chan);
 
        if (ast_parseable_goto(chan, label)) {
@@ -630,7 +636,7 @@ static int gosub_exec(struct ast_channel *chan, const char *data)
        }
 
        /* Create the return address */
-       newframe = gosub_allocate_frame(orig_context, orig_exten, orig_priority + 1, max_argc);
+       newframe = gosub_allocate_frame(orig_context, orig_exten, orig_priority + 1, orig_in_subroutine, max_argc);
        if (!newframe) {
                goto error_exit_locked;
        }
@@ -644,6 +650,8 @@ static int gosub_exec(struct ast_channel *chan, const char *data)
        snprintf(argname, sizeof(argname), "%u", args2.argc);
        frame_set_var(chan, newframe, "ARGC", argname);
 
+       ast_set_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
+
        /* And finally, save our return address */
        AST_LIST_LOCK(oldlist);
        AST_LIST_INSERT_HEAD(oldlist, newframe, entries);
@@ -967,6 +975,7 @@ static int gosub_run(struct ast_channel *chan, const char *sub_args, int ignore_
        int saved_priority;
        int saved_hangup_flags;
        int saved_autoloopflag;
+       int saved_in_subroutine;
        int res;
 
        ast_channel_lock(chan);
@@ -990,6 +999,9 @@ static int gosub_run(struct ast_channel *chan, const char *sub_args, int ignore_
        saved_exten = ast_strdupa(ast_channel_exten(chan));
        saved_priority = ast_channel_priority(chan);
 
+       /* Save whether or not we are in a subroutine */
+       saved_in_subroutine = ast_test_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
+
        ast_debug(4, "%s Original location: %s,%s,%d\n", ast_channel_name(chan),
                saved_context, saved_exten, saved_priority);
 
@@ -1091,6 +1103,9 @@ static int gosub_run(struct ast_channel *chan, const char *sub_args, int ignore_
        /* Restore autoloop flag */
        ast_set2_flag(ast_channel_flags(chan), saved_autoloopflag, AST_FLAG_IN_AUTOLOOP);
 
+       /* Restore subroutine flag */
+       ast_set2_flag(ast_channel_flags(chan), saved_in_subroutine, AST_FLAG_SUBROUTINE_EXEC);
+
        /* Restore non-hangup softhangup flags. */
        if (saved_hangup_flags) {
                ast_softhangup_nolock(chan, saved_hangup_flags);
@@ -1106,6 +1121,7 @@ static int handle_gosub(struct ast_channel *chan, AGI *agi, int argc, const char
        int res;
        int priority;
        int old_autoloopflag;
+       int old_in_subroutine;
        int old_priority;
        const char *old_context;
        const char *old_extension;
@@ -1154,6 +1170,9 @@ static int handle_gosub(struct ast_channel *chan, AGI *agi, int argc, const char
        old_autoloopflag = ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP);
        ast_set_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP);
 
+       /* Save subroutine flag */
+       old_in_subroutine = ast_test_flag(ast_channel_flags(chan), AST_FLAG_SUBROUTINE_EXEC);
+
        /* Save previous location, since we're going to change it */
        old_context = ast_strdupa(ast_channel_context(chan));
        old_extension = ast_strdupa(ast_channel_exten(chan));
@@ -1246,6 +1265,9 @@ static int handle_gosub(struct ast_channel *chan, AGI *agi, int argc, const char
 
        /* Restore autoloop flag */
        ast_set2_flag(ast_channel_flags(chan), old_autoloopflag, AST_FLAG_IN_AUTOLOOP);
+
+       /* Restore subroutine flag */
+       ast_set2_flag(ast_channel_flags(chan), old_in_subroutine, AST_FLAG_SUBROUTINE_EXEC);
        ast_channel_unlock(chan);
 
        return RESULT_SUCCESS;

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index e2d6b7323433c1a2e23105d21bcd79541f01a356..6c41e2751e7bd28afb31720964abafb2cd9d07bf 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -962,6 +962,10 @@ enum {
         * The data on chan->timingdata is an astobj2 object.
         */
        AST_FLAG_TIMINGDATA_IS_AO2_OBJ = (1 << 26),
+       /*!
+        * The channel is executing a subroutine or macro
+        */
+       AST_FLAG_SUBROUTINE_EXEC = (1 << 27),
 };
 
 /*! \brief ast_bridge_config flags */

diff --git a/main/cdr.c b/main/cdr.c
index 1eb2e54698a02acea76cb55640872fea752bfa98..1c8b59b149b44c3be68124bbff262db4f4d4687b 100644 (file)
--- a/main/cdr.c
+++ b/main/cdr.c
@@ -692,6 +692,8 @@ struct cdr_object {
                AST_STRING_FIELD(bridge);           /*!< The bridge the party A happens to be in. */
                AST_STRING_FIELD(appl);             /*!< The last accepted application party A was in */
                AST_STRING_FIELD(data);             /*!< The data for the last accepted application party A was in */
+               AST_STRING_FIELD(context);          /*!< The accepted context for Party A */
+               AST_STRING_FIELD(exten);            /*!< The accepted extension for Party A */
        );
        struct cdr_object *next;                /*!< The next CDR object in the chain */
        struct cdr_object *last;                /*!< The last CDR object in the chain */
@@ -1115,8 +1117,8 @@ static struct ast_cdr *cdr_object_create_public_records(struct cdr_object *cdr)
                ast_copy_string(cdr_copy->uniqueid, party_a->uniqueid, sizeof(cdr_copy->uniqueid));
                ast_copy_string(cdr_copy->lastapp, it_cdr->appl, sizeof(cdr_copy->lastapp));
                ast_copy_string(cdr_copy->lastdata, it_cdr->data, sizeof(cdr_copy->lastdata));
-               ast_copy_string(cdr_copy->dst, party_a->exten, sizeof(cdr_copy->dst));
-               ast_copy_string(cdr_copy->dcontext, party_a->context, sizeof(cdr_copy->dcontext));
+               ast_copy_string(cdr_copy->dst, it_cdr->exten, sizeof(cdr_copy->dst));
+               ast_copy_string(cdr_copy->dcontext, it_cdr->context, sizeof(cdr_copy->dcontext));
 
                /* Party B */
                if (party_b) {
@@ -1353,6 +1355,17 @@ static int base_process_party_a(struct cdr_object *cdr, struct ast_channel_snaps
                cdr_object_check_party_a_hangup(cdr);
                return 0;
        }
+
+       /*
+        * Only record the context and extension if we aren't in a subroutine, or if
+        * we are executing hangup logic.
+        */
+       if (!ast_test_flag(&snapshot->flags, AST_FLAG_SUBROUTINE_EXEC)
+               || ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)) {
+               ast_string_field_set(cdr, context, snapshot->context);
+               ast_string_field_set(cdr, exten, snapshot->exten);
+       }
+
        cdr_object_swap_snapshot(&cdr->party_a, snapshot);
 
        /* When Party A is originated to an application and the application exits, the stack