aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGuy Harris <guy@alum.mit.edu>2015-12-14 11:24:53 -0800
committerGuy Harris <guy@alum.mit.edu>2015-12-14 19:26:11 +0000
commit56584b52e09ae806d85210c1f16f586db1c0d9bd (patch)
tree952b9da4119ab45193507079774d3d7154052eb1
parent5625b62aa417c23102f254268563995773582e39 (diff)
Add comments to explain what we're doing with strings.
Change-Id: I043d02092464ec8cbbec08d11b29dfee248116bf Reviewed-on: https://code.wireshark.org/review/12629 Reviewed-by: Guy Harris <guy@alum.mit.edu>
-rw-r--r--epan/wslua/wslua_pref.c56
1 files changed, 47 insertions, 9 deletions
diff --git a/epan/wslua/wslua_pref.c b/epan/wslua/wslua_pref.c
index 8f45bdee25..d9cf19b9d6 100644
--- a/epan/wslua/wslua_pref.c
+++ b/epan/wslua/wslua_pref.c
@@ -119,6 +119,27 @@ static int new_pref(lua_State* L, pref_type_t type) {
}
case PREF_STRING: {
gchar* def = g_strdup(luaL_optstring(L,2,""));
+ /*
+ * prefs_register_string_preference() assumes that the
+ * variable for the preference points to a static
+ * string that is the initial (default) value of the
+ * preference. It makes a g_strdup()ed copy of that
+ * string, and assigns a pointer to that string to
+ * the variable.
+ *
+ * Our default string is *not* a static string, it's
+ * a g_strdup()ed copy of a string from Lua, so it would
+ * be leaked.
+ *
+ * We save it in info.default_s, as well as setting the
+ * initial value of the preference from it, so that we
+ * can free it after prefs_register_string_preference()
+ * returns.
+ *
+ * (Would that we were programming in a language where
+ * the details of memory management were handled by the
+ * compiler and language support....)
+ */
pref->value.s = def;
pref->info.default_s = def;
break;
@@ -236,20 +257,33 @@ static int Pref__gc(lua_State* L) {
Pref pref = toPref(L,1);
/*
- * Only free unregistered and deregistered Pref; those have
- * a null name pointer.
+ * Only free never-registered and registered-and-then-deregistered
+ * Prefs; those have a null name pointer.
*
- * They've been deregistered by wslua_deregister_protocols(),
- * which means that prefs_deregister_protocol() has been called
- * on them, which means that if their value is something that's
- * allocated, it's been freed, and we don't have to do that
- * ourselves.
+ * If this has never been registered, it obviously has not been
+ * deregistered, so, if it's a string preference, we need to
+ * free the initial value in pref->info.default_s. We don't
+ * need to free the current value, as that's the same string
+ * as the initial value.
+ *
+ * If this has been registred and deregistered, and the current
+ * value was allocated, it was freed when it was deregistered,
+ * so we don't need to free it. If it's a string preference,
+ * the initial value was freed and the pointer to it set to
+ * NULL, so we can still call g_free() on it, as that won't
+ * do anything.
*/
if (! pref->name) {
g_free(pref->label);
g_free(pref->desc);
- if (pref->type == PREF_STRING)
- g_free(pref->info.default_s);
+ if (pref->type == PREF_STRING) {
+ /*
+ * Free the initial string value; if it's not NULL, that
+ * means this is a never-registered protocol, so the
+ * initial value hasn't been freed.
+ */
+ g_free(pref->info.default_s);
+ }
g_free(pref);
}
@@ -360,6 +394,10 @@ WSLUA_METAMETHOD Prefs__newindex(lua_State* L) {
pref->label,
pref->desc,
(const char **)(&(pref->value.s)));
+ /*
+ * We're finished with the initial string value; see
+ * the comment in new_pref().
+ */
g_free(pref->info.default_s);
pref->info.default_s = NULL;
break;