<p>Kevin Harwell <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18747">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pbx.c: Simplify ast_context memory management.<br><br>Allocate all of the ast_context's character data in the structure's<br>flexible array member and eliminate the clunky fake_context. This will<br>simplify future changes to ast_context.<br><br>Change-Id: I98357de75d8ac2b3c4c9f201223632e6901021ea<br>---<br>M main/pbx.c<br>1 file changed, 48 insertions(+), 49 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/pbx.c b/main/pbx.c</span><br><span>index 3a701aa..c655f1b 100644</span><br><span>--- a/main/pbx.c</span><br><span>+++ b/main/pbx.c</span><br><span>@@ -280,21 +280,29 @@</span><br><span> struct ast_exten *exten;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! \brief ast_context: An extension context - must remain in sync with fake_context */</span><br><span style="color: hsl(120, 100%, 40%);">+/*! \brief ast_context: An extension context */</span><br><span> struct ast_context {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_rwlock_t lock; /*!< A lock to prevent multiple threads from clobbering the context */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_exten *root; /*!< The root of the list of extensions */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_hashtab *root_table; /*!< For exact matches on the extensions in the pattern tree, and for traversals of the pattern_tree */</span><br><span style="color: hsl(0, 100%, 40%);">- struct match_char *pattern_tree; /*!< A tree to speed up extension pattern matching */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_context *next; /*!< Link them together */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_includes includes; /*!< Include other contexts */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_ignorepats ignorepats; /*!< Patterns for which to continue playing dialtone */</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_sws alts; /*!< Alternative switches */</span><br><span style="color: hsl(0, 100%, 40%);">- char *registrar; /*!< Registrar -- make sure you malloc this, as the registrar may have to survive module unloads */</span><br><span style="color: hsl(0, 100%, 40%);">- int refcount; /*!< each module that would have created this context should inc/dec this as appropriate */</span><br><span style="color: hsl(0, 100%, 40%);">- int autohints; /*!< Whether autohints support is enabled or not */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_mutex_t macrolock; /*!< A lock to implement "exclusive" macros - held whilst a call is executing in the macro */</span><br><span style="color: hsl(0, 100%, 40%);">- char name[0]; /*!< Name of the context */</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *name;</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *registrar;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rwlock_t lock; /*!< A lock to prevent multiple threads from clobbering the context */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_exten *root; /*!< The root of the list of extensions */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_hashtab *root_table; /*!< For exact matches on the extensions in the pattern tree, and for traversals of the pattern_tree */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct match_char *pattern_tree; /*!< A tree to speed up extension pattern matching */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_context *next; /*!< Link them together */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_includes includes; /*!< Include other contexts */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_ignorepats ignorepats; /*!< Patterns for which to continue playing dialtone */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_sws alts; /*!< Alternative switches */</span><br><span style="color: hsl(120, 100%, 40%);">+ int refcount; /*!< each module that would have created this context should inc/dec this as appropriate */</span><br><span style="color: hsl(120, 100%, 40%);">+ int autohints; /*!< Whether autohints support is enabled or not */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_mutex_t macrolock; /*!< A lock to implement "exclusive" macros - held whilst a call is executing in the macro */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * Buffer to hold the name & registrar character data.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * The context name *must* be stored first in this buffer.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ char data[];</span><br><span> };</span><br><span> </span><br><span> /*! \brief ast_state_cb: An extension state notify register item */</span><br><span>@@ -2429,35 +2437,18 @@</span><br><span> return extension_match_core(pattern, data, needmore);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* This structure must remain in sync with ast_context for proper hashtab matching */</span><br><span style="color: hsl(0, 100%, 40%);">-struct fake_context /* this struct is purely for matching in the hashtab */</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- ast_rwlock_t lock;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_exten *root;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_hashtab *root_table;</span><br><span style="color: hsl(0, 100%, 40%);">- struct match_char *pattern_tree;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_context *next;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_includes includes;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_ignorepats ignorepats;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_sws alts;</span><br><span style="color: hsl(0, 100%, 40%);">- const char *registrar;</span><br><span style="color: hsl(0, 100%, 40%);">- int refcount;</span><br><span style="color: hsl(0, 100%, 40%);">- int autohints;</span><br><span style="color: hsl(0, 100%, 40%);">- ast_mutex_t macrolock;</span><br><span style="color: hsl(0, 100%, 40%);">- char name[256];</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> struct ast_context *ast_context_find(const char *name)</span><br><span> {</span><br><span> struct ast_context *tmp;</span><br><span style="color: hsl(0, 100%, 40%);">- struct fake_context item;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_context item = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .name = name,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span> </span><br><span> if (!name) {</span><br><span> return NULL;</span><br><span> }</span><br><span> ast_rdlock_contexts();</span><br><span> if (contexts_table) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_string(item.name, name, sizeof(item.name));</span><br><span> tmp = ast_hashtab_lookup(contexts_table, &item);</span><br><span> } else {</span><br><span> tmp = NULL;</span><br><span>@@ -2768,7 +2759,10 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- q->incstack[q->stacklen++] = tmp->name; /* Setup the stack */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Technically we should be using tmp->name here, but if we used that we</span><br><span style="color: hsl(120, 100%, 40%);">+ * would have to cast away the constness of the 'name' pointer and I do</span><br><span style="color: hsl(120, 100%, 40%);">+ * not want to do that. */</span><br><span style="color: hsl(120, 100%, 40%);">+ q->incstack[q->stacklen++] = tmp->data; /* Setup the stack */</span><br><span> /* Now try any includes we have in this context */</span><br><span> for (idx = 0; idx < ast_context_includes_count(tmp); idx++) {</span><br><span> const struct ast_include *i = ast_context_includes_get(tmp, idx);</span><br><span>@@ -4806,9 +4800,9 @@</span><br><span> */</span><br><span> static struct ast_context *find_context(const char *context)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct fake_context item;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_string(item.name, context, sizeof(item.name));</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_context item = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .name = context,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span> </span><br><span> return ast_hashtab_lookup(contexts_table, &item);</span><br><span> }</span><br><span>@@ -4821,9 +4815,9 @@</span><br><span> static struct ast_context *find_context_locked(const char *context)</span><br><span> {</span><br><span> struct ast_context *c;</span><br><span style="color: hsl(0, 100%, 40%);">- struct fake_context item;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_string(item.name, context, sizeof(item.name));</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_context item = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .name = context,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span> </span><br><span> ast_rdlock_contexts();</span><br><span> c = ast_hashtab_lookup(contexts_table, &item);</span><br><span>@@ -6199,8 +6193,12 @@</span><br><span> struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *name, const char *registrar)</span><br><span> {</span><br><span> struct ast_context *tmp, **local_contexts;</span><br><span style="color: hsl(0, 100%, 40%);">- struct fake_context search;</span><br><span style="color: hsl(0, 100%, 40%);">- int length = sizeof(struct ast_context) + strlen(name) + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_context search = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .name = name,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t name_bytes = strlen(name);</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t registrar_bytes = strlen(registrar);</span><br><span style="color: hsl(120, 100%, 40%);">+ int length = sizeof(struct ast_context) + name_bytes + registrar_bytes + 2;</span><br><span> </span><br><span> if (!contexts_table) {</span><br><span> /* Protect creation of contexts_table from reentrancy. */</span><br><span>@@ -6216,7 +6214,6 @@</span><br><span> ast_unlock_contexts();</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_string(search.name, name, sizeof(search.name));</span><br><span> if (!extcontexts) {</span><br><span> ast_rdlock_contexts();</span><br><span> local_contexts = &contexts;</span><br><span>@@ -6238,14 +6235,19 @@</span><br><span> if ((tmp = ast_calloc(1, length))) {</span><br><span> ast_rwlock_init(&tmp->lock);</span><br><span> ast_mutex_init(&tmp->macrolock);</span><br><span style="color: hsl(0, 100%, 40%);">- strcpy(tmp->name, name);</span><br><span style="color: hsl(120, 100%, 40%);">+ tmp->name = memcpy(&tmp->data[0], name, name_bytes);</span><br><span style="color: hsl(120, 100%, 40%);">+ tmp->registrar = memcpy(&tmp->data[name_bytes + 1], registrar, registrar_bytes);</span><br><span> tmp->root = NULL;</span><br><span> tmp->root_table = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- tmp->registrar = ast_strdup(registrar);</span><br><span> AST_VECTOR_INIT(&tmp->includes, 0);</span><br><span> AST_VECTOR_INIT(&tmp->ignorepats, 0);</span><br><span> AST_VECTOR_INIT(&tmp->alts, 0);</span><br><span> tmp->refcount = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The context 'name' must be stored at the beginning of 'data.' The</span><br><span style="color: hsl(120, 100%, 40%);">+ * order of subsequent strings (currently only 'registrar') is not</span><br><span style="color: hsl(120, 100%, 40%);">+ * relevant. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_assert(tmp->name == &tmp->data[0]);</span><br><span> } else {</span><br><span> ast_log(LOG_ERROR, "Danger! We failed to allocate a context for %s!\n", name);</span><br><span> if (!extcontexts) {</span><br><span>@@ -8072,9 +8074,6 @@</span><br><span> AST_VECTOR_CALLBACK_VOID(&tmp->alts, sw_free);</span><br><span> AST_VECTOR_FREE(&tmp->alts);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (tmp->registrar)</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(tmp->registrar);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* destroy the hash tabs */</span><br><span> if (tmp->root_table) {</span><br><span> ast_hashtab_destroy(tmp->root_table, 0);</span><br><span></span><br></pre><div style="white-space:pre-wrap"></div><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18747">change 18747</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18747"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I98357de75d8ac2b3c4c9f201223632e6901021ea </div>
<div style="display:none"> Gerrit-Change-Number: 18747 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>