diff options
author | Guy Harris <guy@alum.mit.edu> | 2015-12-14 11:24:53 -0800 |
---|---|---|
committer | Guy Harris <guy@alum.mit.edu> | 2015-12-14 19:26:11 +0000 |
commit | 56584b52e09ae806d85210c1f16f586db1c0d9bd (patch) | |
tree | 952b9da4119ab45193507079774d3d7154052eb1 | |
parent | 5625b62aa417c23102f254268563995773582e39 (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.c | 56 |
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; |