patch 9.2.0257: unnecessary memory allocation in set_callback()

Problem:  Unnecessary memory allocation in set_callback(); after
          set_callback(), callers must manually free the source
          callback's name if cb_free_name is set.
Solution: Refactor set_callback() to re-use the callback name when
          possible to avoid extra memory allocations and clean up so the
          callers do not have to take care themselves (Shane Harper).

closes: #19831

Signed-off-by: Shane Harper <shane@shaneharper.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/change.c b/src/change.c
index ed6f442a232ec08a951be4034b722cd592c5eebb..3a4fefb2b825963aa2f695e4bf8a192e71d01ca0 100644 (file)
--- a/src/change.c
+++ b/src/change.c
@@ -373,8 +373,6 @@ f_listener_add(typval_T *argvars, typval_T *rettv)
     }
 
     set_callback(&lnr->lr_callback, &callback);
-    if (callback.cb_free_name)
-       vim_free(callback.cb_name);
 
     lnr->lr_id = ++next_listener_id;
     rettv->vval.v_number = lnr->lr_id;

diff --git a/src/clipboard.c b/src/clipboard.c
index e869ae63e9699e5029085b0ecb69615008fb0dec..454b686092b3e28b658d520c302bfab2f058070d 100644 (file)
--- a/src/clipboard.c
+++ b/src/clipboard.c
@@ -3789,8 +3789,6 @@ clip_provider_get_callback(
 
     // func_tv owns the function name, so we must make a copy for the callback
     set_callback(callback, &cb);
-    if (cb.cb_free_name)
-       vim_free(cb.cb_name);
     clear_tv(&func_tv);
     return OK;
 }

diff --git a/src/evalvars.c b/src/evalvars.c
index 86bbf1860dbcf964d7fb3de2f9d75de98b0c50a1..1df4af99728a654a1825fcb7f842f6c1336672c2 100644 (file)
--- a/src/evalvars.c
+++ b/src/evalvars.c
@@ -5289,6 +5289,13 @@ put_callback(callback_T *cb, typval_T *tv)
     }
 }
 
+    static bool
+does_callback_own_cb_name(callback_T *cb)
+{
+    // If cb_partial != NULL then *cb->cb_name is owned by the partial.
+    return cb->cb_partial || cb->cb_free_name;
+}
+
 /*
  * Make a copy of "src" into "dest", allocating the function name if needed,
  * without incrementing the refcount.
@@ -5296,19 +5303,13 @@ put_callback(callback_T *cb, typval_T *tv)
     void
 set_callback(callback_T *dest, callback_T *src)
 {
-    if (src->cb_partial == NULL)
+    *dest = *src;
+    if (!does_callback_own_cb_name(src))
     {
-       // just a function name, make a copy
        dest->cb_name = vim_strsave(src->cb_name);
        dest->cb_free_name = TRUE;
     }
-    else
-    {
-       // cb_name is a pointer into cb_partial
-       dest->cb_name = src->cb_name;
-       dest->cb_free_name = FALSE;
-    }
-    dest->cb_partial = src->cb_partial;
+    *src = (callback_T){0};
 }
 
 /*

diff --git a/src/job.c b/src/job.c
index c81421aeb887ce9dfe630571a80f58fa8e00242d..041a8e9b93531a5b971b04ee2398e1f79cb48977 100644 (file)
--- a/src/job.c
+++ b/src/job.c
@@ -1699,8 +1699,6 @@ f_prompt_setcallback(typval_T *argvars, typval_T *rettv UNUSED)
 
     free_callback(&buf->b_prompt_callback);
     set_callback(&buf->b_prompt_callback, &callback);
-    if (callback.cb_free_name)
-       vim_free(callback.cb_name);
 }
 
 /*
@@ -1728,8 +1726,6 @@ f_prompt_setinterrupt(typval_T *argvars, typval_T *rettv UNUSED)
 
     free_callback(&buf->b_prompt_interrupt);
     set_callback(&buf->b_prompt_interrupt, &callback);
-    if (callback.cb_free_name)
-       vim_free(callback.cb_name);
 }
 
 

diff --git a/src/option.c b/src/option.c
index 45446529935eb7ce97c10ffb91069ea86c3036b9..c369fdbf0eaaca8cd78ef21c4cd491c3b3a3ddea 100644 (file)
--- a/src/option.c
+++ b/src/option.c
@@ -9239,8 +9239,6 @@ option_set_callback_func(char_u *optval UNUSED, callback_T *optcb UNUSED)
 
     free_callback(optcb);
     set_callback(optcb, &cb);
-    if (cb.cb_free_name)
-       vim_free(cb.cb_name);
     free_tv(tv);
 
     char_u  *dot = NULL;

diff --git a/src/popupwin.c b/src/popupwin.c
index 2188305debc87b8c07439bd1fd408998b496538d..f14e970e953ddff3a2cb97939630b5f43745e514 100644 (file)
--- a/src/popupwin.c
+++ b/src/popupwin.c
@@ -1037,8 +1037,6 @@ apply_general_options(win_T *wp, dict_T *dict)
        {
            free_callback(&wp->w_filter_cb);
            set_callback(&wp->w_filter_cb, &callback);
-           if (callback.cb_free_name)
-               vim_free(callback.cb_name);
        }
     }
     nr = dict_get_bool(dict, "mapping", -1);
@@ -1069,9 +1067,6 @@ apply_general_options(win_T *wp, dict_T *dict)
 
     free_callback(&wp->w_close_cb);
     set_callback(&wp->w_close_cb, &callback);
-    if (callback.cb_free_name)
-       vim_free(callback.cb_name);
-
     return OK;
 }
 
@@ -2527,8 +2522,6 @@ popup_create(typval_T *argvars, typval_T *rettv, create_type_T type)
        if (callback.cb_name != NULL)
        {
            set_callback(&wp->w_filter_cb, &callback);
-           if (callback.cb_free_name)
-               vim_free(callback.cb_name);
        }
 
        wp->w_p_wrap = 0;

diff --git a/src/quickfix.c b/src/quickfix.c
index ed82c61616a66d2567c6dd7b1d1cd2cd563e2aaa..59579171c398fe562e8c396e414c52b410784308 100644 (file)
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -8158,11 +8158,7 @@ qf_setprop_qftf(qf_info_T *qi UNUSED, qf_list_T *qfl, dictitem_T *di)
     cb = get_callback(&di->di_tv);
     if (cb.cb_name == NULL || *cb.cb_name == NUL)
        return OK;
-
     set_callback(&qfl->qf_qftf_cb, &cb);
-    if (cb.cb_free_name)
-       vim_free(cb.cb_name);
-
     return OK;
 }
 

diff --git a/src/sound.c b/src/sound.c
index 7e7d434c59887ca91075ddd538f5e9de95019b52..4e5acb5d473789a8a7a379de7d1b26c6f2668d67 100644 (file)
--- a/src/sound.c
+++ b/src/sound.c
@@ -63,8 +63,6 @@ get_sound_callback(typval_T *arg)
     soundcb->snd_next = first_callback;
     first_callback = soundcb;
     set_callback(&soundcb->snd_callback, &callback);
-    if (callback.cb_free_name)
-        vim_free(callback.cb_name);
     return soundcb;
 }
 

diff --git a/src/time.c b/src/time.c
index 7b91a7c01c7dfdf02ad994a724157f78bdb86a15..16112d6eeb411895faddf1f01c8c4cc5a6874ed1 100644 (file)
--- a/src/time.c
+++ b/src/time.c
@@ -912,8 +912,6 @@ f_timer_start(typval_T *argvars, typval_T *rettv)
        return;
     }
     set_callback(&timer->tr_callback, &callback);
-    if (callback.cb_free_name)
-       vim_free(callback.cb_name);
     rettv->vval.v_number = (varnumber_T)timer->tr_id;
 }
 

diff --git a/src/version.c b/src/version.c
index 5dbd0f40178b0cba46f733cdd9bee7fe2cae7065..770e09b8609f8f476a88ef53d226d0494488c530 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    257,
 /**/
     256,
 /**/