From 1436d8d51cce114be56209924fc71376407e5bad Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Mon, 11 Jul 2016 22:41:15 +0200 Subject: [PATCH 1/3] patch 7.4.2026 Problem: Reference counting for callbacks isn't right. Solution: Add free_callback(). (Ken Takata) Fix reference count. --- src/channel.c | 106 +++++++++++++++------------------------------ src/eval.c | 22 +++++++++- src/ex_cmds2.c | 3 +- src/proto/eval.pro | 1 + src/version.c | 2 + 5 files changed, 61 insertions(+), 73 deletions(-) diff --git a/src/channel.c b/src/channel.c index 2e88080414..90ef9741af 100644 --- a/src/channel.c +++ b/src/channel.c @@ -1113,6 +1113,23 @@ find_buffer(char_u *name, int err) return buf; } + static void +set_callback( + char_u **cbp, + partial_T **pp, + char_u *callback, + partial_T *partial) +{ + free_callback(*cbp, *pp); + if (callback != NULL && *callback != NUL) + *cbp = vim_strsave(callback); + else + *cbp = NULL; + *pp = partial; + if (*pp != NULL) + ++(*pp)->pt_refcount; +} + /* * Set various properties from an "opt" argument. */ @@ -1120,8 +1137,6 @@ find_buffer(char_u *name, int err) channel_set_options(channel_T *channel, jobopt_T *opt) { int part; - char_u **cbp; - partial_T **pp; if (opt->jo_set & JO_MODE) for (part = PART_SOCK; part <= PART_IN; ++part) @@ -1144,61 +1159,19 @@ channel_set_options(channel_T *channel, jobopt_T *opt) channel->ch_part[PART_IN].ch_block_write = 1; if (opt->jo_set & JO_CALLBACK) - { - cbp = &channel->ch_callback; - pp = &channel->ch_partial; - vim_free(*cbp); - partial_unref(*pp); - if (opt->jo_callback != NULL && *opt->jo_callback != NUL) - *cbp = vim_strsave(opt->jo_callback); - else - *cbp = NULL; - *pp = opt->jo_partial; - if (*pp != NULL) - ++(*pp)->pt_refcount; - } + set_callback(&channel->ch_callback, &channel->ch_partial, + opt->jo_callback, opt->jo_partial); if (opt->jo_set & JO_OUT_CALLBACK) - { - cbp = &channel->ch_part[PART_OUT].ch_callback; - pp = &channel->ch_part[PART_OUT].ch_partial; - vim_free(*cbp); - partial_unref(*pp); - if (opt->jo_out_cb != NULL && *opt->jo_out_cb != NUL) - *cbp = vim_strsave(opt->jo_out_cb); - else - *cbp = NULL; - *pp = opt->jo_out_partial; - if (*pp != NULL) - ++(*pp)->pt_refcount; - } + set_callback(&channel->ch_part[PART_OUT].ch_callback, + &channel->ch_part[PART_OUT].ch_partial, + opt->jo_out_cb, opt->jo_out_partial); if (opt->jo_set & JO_ERR_CALLBACK) - { - cbp = &channel->ch_part[PART_ERR].ch_callback; - pp = &channel->ch_part[PART_ERR].ch_partial; - vim_free(*cbp); - partial_unref(*pp); - if (opt->jo_err_cb != NULL && *opt->jo_err_cb != NUL) - *cbp = vim_strsave(opt->jo_err_cb); - else - *cbp = NULL; - *pp = opt->jo_err_partial; - if (*pp != NULL) - ++(*pp)->pt_refcount; - } + set_callback(&channel->ch_part[PART_ERR].ch_callback, + &channel->ch_part[PART_ERR].ch_partial, + opt->jo_err_cb, opt->jo_err_partial); if (opt->jo_set & JO_CLOSE_CALLBACK) - { - cbp = &channel->ch_close_cb; - pp = &channel->ch_close_partial; - vim_free(*cbp); - partial_unref(*pp); - if (opt->jo_close_cb != NULL && *opt->jo_close_cb != NUL) - *cbp = vim_strsave(opt->jo_close_cb); - else - *cbp = NULL; - *pp = opt->jo_close_partial; - if (*pp != NULL) - ++(*pp)->pt_refcount; - } + set_callback(&channel->ch_close_cb, &channel->ch_close_partial, + opt->jo_close_cb, opt->jo_close_partial); if ((opt->jo_set & JO_OUT_IO) && opt->jo_io[PART_OUT] == JIO_BUFFER) { @@ -2228,8 +2201,7 @@ invoke_one_time_callback( * invokes ch_close() the list will be cleared. */ remove_cb_node(cbhead, item); invoke_callback(channel, item->cq_callback, item->cq_partial, argv); - vim_free(item->cq_callback); - partial_unref(item->cq_partial); + free_callback(item->cq_callback, item->cq_partial); vim_free(item); } @@ -2725,9 +2697,8 @@ channel_close(channel_T *channel, int invoke_close_cb) } /* the callback is only called once */ - vim_free(channel->ch_close_cb); + free_callback(channel->ch_close_cb, channel->ch_close_partial); channel->ch_close_cb = NULL; - partial_unref(channel->ch_close_partial); channel->ch_close_partial = NULL; --channel->ch_refcount; @@ -2763,8 +2734,7 @@ channel_clear_one(channel_T *channel, int part) cbq_T *node = cb_head->cq_next; remove_cb_node(cb_head, node); - vim_free(node->cq_callback); - partial_unref(node->cq_partial); + free_callback(node->cq_callback, node->cq_partial); vim_free(node); } @@ -2774,9 +2744,9 @@ channel_clear_one(channel_T *channel, int part) remove_json_node(json_head, json_head->jq_next); } - vim_free(channel->ch_part[part].ch_callback); + free_callback(channel->ch_part[part].ch_callback, + channel->ch_part[part].ch_partial); channel->ch_part[part].ch_callback = NULL; - partial_unref(channel->ch_part[part].ch_partial); channel->ch_part[part].ch_partial = NULL; } @@ -2793,13 +2763,11 @@ channel_clear(channel_T *channel) channel_clear_one(channel, PART_OUT); channel_clear_one(channel, PART_ERR); /* there is no callback or queue for PART_IN */ - vim_free(channel->ch_callback); + free_callback(channel->ch_callback, channel->ch_partial); channel->ch_callback = NULL; - partial_unref(channel->ch_partial); channel->ch_partial = NULL; - vim_free(channel->ch_close_cb); + free_callback(channel->ch_close_cb, channel->ch_close_partial); channel->ch_close_cb = NULL; - partial_unref(channel->ch_close_partial); channel->ch_close_partial = NULL; } @@ -4319,8 +4287,7 @@ job_free_contents(job_T *job) mch_clear_job(job); vim_free(job->jv_stoponexit); - vim_free(job->jv_exit_cb); - partial_unref(job->jv_exit_partial); + free_callback(job->jv_exit_cb, job->jv_exit_partial); } static void @@ -4485,8 +4452,7 @@ job_set_options(job_T *job, jobopt_T *opt) } if (opt->jo_set & JO_EXIT_CB) { - vim_free(job->jv_exit_cb); - partial_unref(job->jv_exit_partial); + free_callback(job->jv_exit_cb, job->jv_exit_partial); if (opt->jo_exit_cb == NULL || *opt->jo_exit_cb == NUL) { job->jv_exit_cb = NULL; diff --git a/src/eval.c b/src/eval.c index a9504363d1..779886431e 100644 --- a/src/eval.c +++ b/src/eval.c @@ -21178,13 +21178,33 @@ get_callback(typval_T *arg, partial_T **pp) return (*pp)->pt_name; } *pp = NULL; - if (arg->v_type == VAR_FUNC || arg->v_type == VAR_STRING) + if (arg->v_type == VAR_FUNC) + { + func_ref(arg->vval.v_string); + return arg->vval.v_string; + } + if (arg->v_type == VAR_STRING) return arg->vval.v_string; if (arg->v_type == VAR_NUMBER && arg->vval.v_number == 0) return (char_u *)""; EMSG(_("E921: Invalid callback argument")); return NULL; } + +/* + * Unref/free "callback" and "partial" retured by get_callback(). + */ + void +free_callback(char_u *callback, partial_T *partial) +{ + if (partial != NULL) + partial_unref(partial); + else if (callback != NULL) + { + func_unref(callback); + vim_free(callback); + } +} #endif #ifdef FEAT_TIMERS diff --git a/src/ex_cmds2.c b/src/ex_cmds2.c index c8980b44b7..2b2d63549f 100644 --- a/src/ex_cmds2.c +++ b/src/ex_cmds2.c @@ -1121,8 +1121,7 @@ remove_timer(timer_T *timer) static void free_timer(timer_T *timer) { - vim_free(timer->tr_callback); - partial_unref(timer->tr_partial); + free_callback(timer->tr_callback, timer->tr_partial); vim_free(timer); } diff --git a/src/proto/eval.pro b/src/proto/eval.pro index e81669b05e..6d1144776b 100644 --- a/src/proto/eval.pro +++ b/src/proto/eval.pro @@ -94,6 +94,7 @@ void mzscheme_call_vim(char_u *name, typval_T *args, typval_T *rettv); float_T vim_round(float_T f); long do_searchpair(char_u *spat, char_u *mpat, char_u *epat, int dir, char_u *skip, int flags, pos_T *match_pos, linenr_T lnum_stop, long time_limit); char_u *get_callback(typval_T *arg, partial_T **pp); +void free_callback(char_u *callback, partial_T *partial); void set_vim_var_nr(int idx, varnumber_T val); varnumber_T get_vim_var_nr(int idx); char_u *get_vim_var_str(int idx); diff --git a/src/version.c b/src/version.c index 67f8b653e1..a7a9a8bd90 100644 --- a/src/version.c +++ b/src/version.c @@ -758,6 +758,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 2026, /**/ 2025, /**/ From 4c06815c44dfeaafdad25dfcc40f60860096a900 Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Mon, 11 Jul 2016 23:15:25 +0200 Subject: [PATCH 2/3] patch 7.4.2027 Problem: Can't build with +eval but without +menu. Solution: Add #ifdef. (John Marriott) --- src/eval.c | 2 ++ src/version.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/eval.c b/src/eval.c index 779886431e..6987485edc 100644 --- a/src/eval.c +++ b/src/eval.c @@ -13120,11 +13120,13 @@ f_getcompletion(typval_T *argvars, typval_T *rettv) return; } +# if defined(FEAT_MENU) if (xpc.xp_context == EXPAND_MENUS) { set_context_in_menu_cmd(&xpc, (char_u *)"menu", xpc.xp_pattern, FALSE); xpc.xp_pattern_len = (int)STRLEN(xpc.xp_pattern); } +# endif pat = addstar(xpc.xp_pattern, xpc.xp_pattern_len, xpc.xp_context); if ((rettv_list_alloc(rettv) != FAIL) && (pat != NULL)) diff --git a/src/version.c b/src/version.c index a7a9a8bd90..93f3c03fa0 100644 --- a/src/version.c +++ b/src/version.c @@ -758,6 +758,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 2027, /**/ 2026, /**/ From 5498a41f5a62c3877fee0185adf3bf7245a9a547 Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Mon, 11 Jul 2016 23:19:05 +0200 Subject: [PATCH 3/3] patch 7.4.2028 Problem: cppcheck warns for using index before limits check. Solution: Swap the expressions. (Dominique Pelle) --- src/mbyte.c | 2 +- src/version.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mbyte.c b/src/mbyte.c index 7bc184ba24..f85d5ad60b 100644 --- a/src/mbyte.c +++ b/src/mbyte.c @@ -4357,7 +4357,7 @@ enc_locale(void) else s = p + 1; } - for (i = 0; s[i] != NUL && i < (int)sizeof(buf) - 1; ++i) + for (i = 0; i < (int)sizeof(buf) - 1 && s[i] != NUL; ++i) { if (s[i] == '_' || s[i] == '-') buf[i] = '-'; diff --git a/src/version.c b/src/version.c index 93f3c03fa0..6d75f76f1d 100644 --- a/src/version.c +++ b/src/version.c @@ -758,6 +758,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 2028, /**/ 2027, /**/