From: John Marriott Date: Fri, 2 Jan 2026 14:11:58 +0000 (+0000) Subject: patch 9.1.2044: Inefficient use of ga_concat() X-Git-Tag: v9.1.2044^0 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7e671fbb9fa2af9ad6c4ba9a7a881df431cd02b;p=thirdparty%2Fvim.git patch 9.1.2044: Inefficient use of ga_concat() Problem: Inefficient use of ga_concat() Solution: Use ga_concat_len() when the length is already known to avoid use of strlen() (John Marriott). Additionally the following changes are done: os_unix.c: - in function `socket_server_list_sockets()` use a `string_T` for the strings `buf` and `path` for use in `ga_concat_len()` and drop un-needed variable `dir`. quickfix.c: - in function `qf_jump_print_msg()` use a `string_T` for the string `IObuff` for use in `ga_concat_len()`. - in function `qf_range_text()` use a `string_T` for the string `buf` for use in `ga_concat_len()`. register.c: - simplify function `execreg_line_continuation()`. terminal.c: - in function `read_dump_file()` use a `string_T` for the string `prev_char` for use in `ga_concat_len()`. tuple.c: - in function `tuple_join_inner()` use a `string_T` for the string `s` for use in `ga_concat_len()`. Also, change local struct `join_T` to use `string_T`. vim9type.c: - in functions `type_name_tuple()` and `type_name_func()` use a `string_T` for the string `arg_type` for use in `ga_concat_len()`. closes: #19038 Signed-off-by: John Marriott Signed-off-by: Christian Brabandt --- diff --git a/src/blob.c b/src/blob.c index 9dfda36fb3..e72b5d00a7 100644 --- a/src/blob.c +++ b/src/blob.c @@ -276,13 +276,16 @@ blob2string(blob_T *blob, char_u **tofree, char_u *numbuf) // Store bytes in the growarray. ga_init2(&ga, 1, 4000); - ga_concat(&ga, (char_u *)"0z"); + ga_concat_len(&ga, (char_u *)"0z", 2); for (i = 0; i < blob_len(blob); i++) { + size_t numbuflen; + if (i > 0 && (i & 3) == 0) - ga_concat(&ga, (char_u *)"."); - vim_snprintf((char *)numbuf, NUMBUFLEN, "%02X", blob_get(blob, i)); - ga_concat(&ga, numbuf); + ga_concat_len(&ga, (char_u *)".", 1); + numbuflen = vim_snprintf_safelen((char *)numbuf, NUMBUFLEN, + "%02X", blob_get(blob, i)); + ga_concat_len(&ga, numbuf, numbuflen); } ga_append(&ga, NUL); // append a NUL at the end *tofree = ga.ga_data; diff --git a/src/eval.c b/src/eval.c index 75418c449d..15ee3fe110 100644 --- a/src/eval.c +++ b/src/eval.c @@ -6270,14 +6270,14 @@ partial_tv2string( fname = string_quote(pt == NULL ? NULL : partial_name(pt), FALSE); ga_init2(&ga, 1, 100); - ga_concat(&ga, (char_u *)"function("); + ga_concat_len(&ga, (char_u *)"function(", 9); if (fname != NULL) { // When using uf_name prepend "g:" for a global function. if (pt != NULL && pt->pt_name == NULL && fname[0] == '\'' && vim_isupper(fname[1])) { - ga_concat(&ga, (char_u *)"'g:"); + ga_concat_len(&ga, (char_u *)"'g:", 3); ga_concat(&ga, fname + 1); } else @@ -6286,21 +6286,21 @@ partial_tv2string( } if (pt != NULL && pt->pt_argc > 0) { - ga_concat(&ga, (char_u *)", ["); + ga_concat_len(&ga, (char_u *)", [", 3); for (i = 0; i < pt->pt_argc; ++i) { if (i > 0) - ga_concat(&ga, (char_u *)", "); + ga_concat_len(&ga, (char_u *)", ", 2); ga_concat(&ga, tv2string(&pt->pt_argv[i], &tf, numbuf, copyID)); vim_free(tf); } - ga_concat(&ga, (char_u *)"]"); + ga_concat_len(&ga, (char_u *)"]", 1); } if (pt != NULL && pt->pt_dict != NULL) { typval_T dtv; - ga_concat(&ga, (char_u *)", "); + ga_concat_len(&ga, (char_u *)", ", 2); dtv.v_type = VAR_DICT; dtv.vval.v_dict = pt->pt_dict; ga_concat(&ga, tv2string(&dtv, &tf, numbuf, copyID)); diff --git a/src/evalwindow.c b/src/evalwindow.c index 9a4efaa8c3..57620b61bb 100644 --- a/src/evalwindow.c +++ b/src/evalwindow.c @@ -1200,10 +1200,14 @@ f_winrestcmd(typval_T *argvars UNUSED, typval_T *rettv) winnr = 1; FOR_ALL_WINDOWS(wp) { - sprintf((char *)buf, ":%dresize %d|", winnr, wp->w_height); - ga_concat(&ga, buf); - sprintf((char *)buf, "vert :%dresize %d|", winnr, wp->w_width); - ga_concat(&ga, buf); + size_t buflen; + + buflen = vim_snprintf_safelen((char *)buf, sizeof(buf), + ":%dresize %d|", winnr, wp->w_height); + ga_concat_len(&ga, buf, buflen); + buflen = vim_snprintf_safelen((char *)buf, sizeof(buf), + "vert :%dresize %d|", winnr, wp->w_width); + ga_concat_len(&ga, buf, buflen); ++winnr; } } diff --git a/src/gui.c b/src/gui.c index 1470facf9c..aeb46c5af5 100644 --- a/src/gui.c +++ b/src/gui.c @@ -5308,26 +5308,26 @@ gui_do_findrepl( ga_init2(&ga, 1, 100); if (type == FRD_REPLACEALL) - ga_concat(&ga, (char_u *)"%s/"); + ga_concat_len(&ga, (char_u *)"%s/", 3); - ga_concat(&ga, (char_u *)"\\V"); + ga_concat_len(&ga, (char_u *)"\\V", 2); if (flags & FRD_MATCH_CASE) - ga_concat(&ga, (char_u *)"\\C"); + ga_concat_len(&ga, (char_u *)"\\C", 2); else - ga_concat(&ga, (char_u *)"\\c"); + ga_concat_len(&ga, (char_u *)"\\c", 2); if (flags & FRD_WHOLE_WORD) - ga_concat(&ga, (char_u *)"\\<"); + ga_concat_len(&ga, (char_u *)"\\<", 2); // escape slash and backslash p = vim_strsave_escaped(find_text, (char_u *)"/\\"); if (p != NULL) ga_concat(&ga, p); vim_free(p); if (flags & FRD_WHOLE_WORD) - ga_concat(&ga, (char_u *)"\\>"); + ga_concat_len(&ga, (char_u *)"\\>", 2); if (type == FRD_REPLACEALL) { - ga_concat(&ga, (char_u *)"/"); + ga_concat_len(&ga, (char_u *)"/", 1); // Escape slash and backslash. // Also escape tilde and ampersand if 'magic' is set. p = vim_strsave_escaped(repl_text, @@ -5335,7 +5335,7 @@ gui_do_findrepl( if (p != NULL) ga_concat(&ga, p); vim_free(p); - ga_concat(&ga, (char_u *)"/g"); + ga_concat_len(&ga, (char_u *)"/g", 2); } ga_append(&ga, NUL); diff --git a/src/os_unix.c b/src/os_unix.c index fdae5d67fa..490be1f254 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -9363,8 +9363,8 @@ socket_server_uninit(void) socket_server_list_sockets(void) { garray_T str; - char_u *buf; - char_u *path; + string_T buf; + string_T path; DIR *dirp; struct dirent *dp; struct sockaddr_un addr; @@ -9374,13 +9374,15 @@ socket_server_list_sockets(void) (char_u *)"/tmp" }; - if ((buf = alloc(sizeof(addr.sun_path))) == NULL) + if ((buf.string = alloc(sizeof(addr.sun_path))) == NULL) return NULL; - if ((path = alloc(sizeof(addr.sun_path))) == NULL) + if ((path.string = alloc(sizeof(addr.sun_path))) == NULL) { - vim_free(buf); + vim_free(buf.string); return NULL; } + buf.length = 0; + path.length = 0; ga_init2(&str, 1, 100); @@ -9393,15 +9395,13 @@ socket_server_list_sockets(void) if (STRCMP(dir, "/tmp") == 0 || (known_dirs[1] != NULL && STRCMP(dir, known_dirs[1]) == 0)) - vim_snprintf((char *)path, sizeof(addr.sun_path), "%s/vim-%lu", - dir, (unsigned long int)getuid()); + path.length = vim_snprintf_safelen((char *)path.string, sizeof(addr.sun_path), + "%s/vim-%lu", dir, (unsigned long int)getuid()); else - vim_snprintf((char *)path, sizeof(addr.sun_path), "%s/vim", dir); - - dir = path; - - dirp = opendir((char *)dir); + path.length = vim_snprintf_safelen((char *)path.string, sizeof(addr.sun_path), + "%s/vim", dir); + dirp = opendir((char *)path.string); if (dirp == NULL) continue; @@ -9411,15 +9411,15 @@ socket_server_list_sockets(void) if (STRCMP(dp->d_name, ".") == 0 || STRCMP(dp->d_name, "..") == 0) continue; - vim_snprintf((char *)buf, sizeof(addr.sun_path), "%s/%s", - dir, dp->d_name); + buf.length = vim_snprintf_safelen((char *)buf.string, sizeof(addr.sun_path), + "%s/%s", path.string, dp->d_name); // Try sending an ALIVE command. This is more assuring than a // simple connect, and *also seems to make tests less flaky*. - if (!socket_server_check_alive(buf)) + if (!socket_server_check_alive(buf.string)) continue; - ga_concat(&str, (char_u *)dp->d_name); + ga_concat_len(&str, (char_u *)dp->d_name, buf.length - (path.length + 1)); ga_append(&str, '\n'); } @@ -9428,8 +9428,8 @@ socket_server_list_sockets(void) break; } - vim_free(path); - vim_free(buf); + vim_free(path.string); + vim_free(buf.string); ga_append(&str, NUL); diff --git a/src/quickfix.c b/src/quickfix.c index c3c1ea84f8..07e0b7aa1b 100644 --- a/src/quickfix.c +++ b/src/quickfix.c @@ -3646,6 +3646,7 @@ qf_jump_print_msg( { linenr_T i; garray_T *gap; + size_t IObufflen; gap = qfga_get(); @@ -3653,12 +3654,13 @@ qf_jump_print_msg( // scrolled up. if (!msg_scrolled) update_topline_redraw(); - vim_snprintf((char *)IObuff, IOSIZE, _("(%d of %d)%s%s: "), qf_index, + IObufflen = vim_snprintf_safelen((char *)IObuff, IOSIZE, + _("(%d of %d)%s%s: "), qf_index, qf_get_curlist(qi)->qf_count, qf_ptr->qf_cleared ? _(" (line deleted)") : "", (char *)qf_types(qf_ptr->qf_type, qf_ptr->qf_nr)); // Add the message, skipping leading whitespace and newlines. - ga_concat(gap, IObuff); + ga_concat_len(gap, IObuff, IObufflen); qf_fmt_text(gap, skipwhite(qf_ptr->qf_text)); ga_append(gap, NUL); @@ -4126,32 +4128,25 @@ qf_fmt_text(garray_T *gap, char_u *text) static void qf_range_text(garray_T *gap, qfline_T *qfp) { - char_u *buf = IObuff; - int bufsize = IOSIZE; - int len; + string_T buf = {IObuff, 0}; - vim_snprintf((char *)buf, bufsize, "%ld", qfp->qf_lnum); - len = (int)STRLEN(buf); + buf.length = vim_snprintf_safelen((char *)buf.string, IOSIZE, "%ld", qfp->qf_lnum); if (qfp->qf_end_lnum > 0 && qfp->qf_lnum != qfp->qf_end_lnum) { - vim_snprintf((char *)buf + len, bufsize - len, "-%ld", - qfp->qf_end_lnum); - len += (int)STRLEN(buf + len); + buf.length += vim_snprintf_safelen((char *)buf.string + buf.length, + IOSIZE - buf.length, "-%ld", qfp->qf_end_lnum); } if (qfp->qf_col > 0) { - vim_snprintf((char *)buf + len, bufsize - len, " col %d", qfp->qf_col); - len += (int)STRLEN(buf + len); + buf.length += vim_snprintf_safelen((char *)buf.string + buf.length, + IOSIZE - buf.length, " col %d", qfp->qf_col); if (qfp->qf_end_col > 0 && qfp->qf_col != qfp->qf_end_col) - { - vim_snprintf((char *)buf + len, bufsize - len, "-%d", - qfp->qf_end_col); - len += (int)STRLEN(buf + len); - } + buf.length += vim_snprintf_safelen((char *)buf.string + buf.length, + IOSIZE - buf.length, "-%d", qfp->qf_end_col); } - ga_concat_len(gap, buf, len); + ga_concat_len(gap, buf.string, buf.length); } /* diff --git a/src/register.c b/src/register.c index 90abbcc54b..a92dd22823 100644 --- a/src/register.c +++ b/src/register.c @@ -565,10 +565,9 @@ set_execreg_lastc(int lastc) execreg_line_continuation(string_T *lines, long *idx) { garray_T ga; - long i = *idx; - char_u *p; - int cmd_start; - int cmd_end = i; + long cmd_start = *idx; + long cmd_end = *idx; + string_T *tmp; int j; char_u *str; @@ -577,19 +576,24 @@ execreg_line_continuation(string_T *lines, long *idx) // search backwards to find the first line of this command. // Any line not starting with \ or "\ is the start of the // command. - while (--i > 0) + while (--cmd_start > 0) { - p = skipwhite(lines[i].string); + char_u *p; + + p = skipwhite(lines[cmd_start].string); if (*p != '\\' && (p[0] != '"' || p[1] != '\\' || p[2] != ' ')) break; } - cmd_start = i; // join all the lines - ga_concat(&ga, lines[cmd_start].string); + tmp = &lines[cmd_start]; + ga_concat_len(&ga, tmp->string, tmp->length); for (j = cmd_start + 1; j <= cmd_end; j++) { - p = skipwhite(lines[j].string); + char_u *p; + + tmp = &lines[j]; + p = skipwhite(tmp->string); if (*p == '\\') { // Adjust the growsize to the current length to @@ -601,14 +605,15 @@ execreg_line_continuation(string_T *lines, long *idx) else ga.ga_growsize = ga.ga_len; } - ga_concat(&ga, p + 1); + p++; + ga_concat_len(&ga, p, (tmp->string + tmp->length) - p); } } ga_append(&ga, NUL); str = vim_strnsave(ga.ga_data, ga.ga_len); ga_clear(&ga); - *idx = i; + *idx = cmd_start; return str; } diff --git a/src/terminal.c b/src/terminal.c index 234807e904..d2e20e251b 100644 --- a/src/terminal.c +++ b/src/terminal.c @@ -5378,7 +5378,7 @@ f_term_dumpwrite(typval_T *argvars, typval_T *rettv UNUSED) static void dump_is_corrupt(garray_T *gap) { - ga_concat(gap, (char_u *)"CORRUPT"); + ga_concat_len(gap, (char_u *)"CORRUPT", 7); } static void @@ -5409,7 +5409,7 @@ read_dump_file(FILE *fd, VTermPos *cursor_pos) int c; garray_T ga_text; garray_T ga_cell; - char_u *prev_char = NULL; + string_T prev_char = {NULL, 0}; int attr = 0; cellattr_T cell; cellattr_T empty_cell; @@ -5488,10 +5488,16 @@ read_dump_file(FILE *fd, VTermPos *cursor_pos) } // save the character for repeating it - vim_free(prev_char); + VIM_CLEAR_STRING(prev_char); if (ga_text.ga_data != NULL) - prev_char = vim_strnsave(((char_u *)ga_text.ga_data) + prev_len, - ga_text.ga_len - prev_len); + { + prev_char.length = (size_t)(ga_text.ga_len - prev_len); + prev_char.string = vim_strnsave( + ((char_u *)ga_text.ga_data) + prev_len, + prev_char.length); + if (prev_char.string == NULL) + prev_char.length = 0; + } if (c == '@' || c == '|' || c == '>' || c == '\n') { @@ -5601,7 +5607,7 @@ read_dump_file(FILE *fd, VTermPos *cursor_pos) } else if (c == '@') { - if (prev_char == NULL) + if (prev_char.string == NULL) dump_is_corrupt(&ga_text); else { @@ -5618,7 +5624,7 @@ read_dump_file(FILE *fd, VTermPos *cursor_pos) while (count-- > 0) { - ga_concat(&ga_text, prev_char); + ga_concat_len(&ga_text, prev_char.string, prev_char.length); append_cell(&ga_cell, &cell); } } @@ -5641,7 +5647,7 @@ read_dump_file(FILE *fd, VTermPos *cursor_pos) ga_clear(&ga_text); ga_clear(&ga_cell); - vim_free(prev_char); + vim_free(prev_char.string); return max_cells; } diff --git a/src/tuple.c b/src/tuple.c index c58d11d7a1..28fb598883 100644 --- a/src/tuple.c +++ b/src/tuple.c @@ -633,7 +633,7 @@ tuple_lock(tuple_T *tuple, int deep, int lock, int check_refcount) } typedef struct join_S { - char_u *s; + string_T s; char_u *tofree; } join_T; @@ -649,37 +649,39 @@ tuple_join_inner( { int i; join_T *p; - int len; int sumlen = 0; int first = TRUE; char_u *tofree; char_u numbuf[NUMBUFLEN]; - char_u *s; + string_T s; typval_T *tv; + size_t seplen = 0; // Stringify each item in the tuple. for (i = 0; i < TUPLE_LEN(tuple) && !got_int; i++) { tv = TUPLE_ITEM(tuple, i); - s = echo_string_core(tv, &tofree, numbuf, copyID, + s.string = echo_string_core(tv, &tofree, numbuf, copyID, echo_style, restore_copyID, !echo_style); - if (s == NULL) + if (s.string == NULL) return FAIL; - len = (int)STRLEN(s); - sumlen += len; + s.length = STRLEN(s.string); + sumlen += (int)s.length; (void)ga_grow(join_gap, 1); p = ((join_T *)join_gap->ga_data) + (join_gap->ga_len++); - if (tofree != NULL || s != numbuf) + if (tofree != NULL || s.string != numbuf) { - p->s = s; + p->s.string = s.string; + p->s.length = s.length; p->tofree = tofree; } else { - p->s = vim_strnsave(s, len); - p->tofree = p->s; + p->s.string = vim_strnsave(s.string, s.length); + p->s.length = s.length; + p->tofree = p->s.string; } line_breakcheck(); @@ -689,8 +691,12 @@ tuple_join_inner( // Allocate result buffer with its total size, avoid re-allocation and // multiple copy operations. Add 2 for a tailing ')' and NUL. - if (join_gap->ga_len >= 2) - sumlen += (int)STRLEN(sep) * (join_gap->ga_len - 1); + if (join_gap->ga_len > 0) + { + seplen = STRLEN(sep); + if (join_gap->ga_len >= 2) + sumlen += (int)seplen * (join_gap->ga_len - 1); + } if (ga_grow(gap, sumlen + 2) == FAIL) return FAIL; @@ -699,18 +705,18 @@ tuple_join_inner( if (first) first = FALSE; else - ga_concat(gap, sep); + ga_concat_len(gap, sep, seplen); p = ((join_T *)join_gap->ga_data) + i; - if (p->s != NULL) - ga_concat(gap, p->s); + if (p->s.string != NULL) + ga_concat_len(gap, p->s.string, p->s.length); line_breakcheck(); } // If there is only one item in the tuple, then add the separator after // that. if (join_gap->ga_len == 1) - ga_concat(gap, sep); + ga_concat_len(gap, sep, seplen); return OK; } diff --git a/src/version.c b/src/version.c index 84a08d2bec..ca2125168c 100644 --- 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 */ +/**/ + 2044, /**/ 2043, /**/ diff --git a/src/vim9type.c b/src/vim9type.c index 6d1b28afdc..393b852514 100644 --- a/src/vim9type.c +++ b/src/vim9type.c @@ -2580,30 +2580,29 @@ type_name_tuple(type_T *type, char **tofree) if (type->tt_argcount <= 0) // empty tuple - ga_concat(&ga, (char_u *)"any"); + ga_concat_len(&ga, (char_u *)"any", 3); else { if (type->tt_args == NULL) - ga_concat(&ga, (char_u *)"[unknown]"); + ga_concat_len(&ga, (char_u *)"[unknown]", 9); else { for (i = 0; i < type->tt_argcount; ++i) { - char *arg_type; - int len; + string_T arg_type; - arg_type = type_name(type->tt_args[i], &arg_free); + arg_type.string = (char_u *)type_name(type->tt_args[i], &arg_free); if (i > 0) { STRCPY((char *)ga.ga_data + ga.ga_len, ", "); ga.ga_len += 2; } - len = (int)STRLEN(arg_type); - if (ga_grow(&ga, len + 8) == FAIL) + arg_type.length = STRLEN(arg_type.string); + if (ga_grow(&ga, (int)arg_type.length + 8) == FAIL) goto failed; if (varargs && i == type->tt_argcount - 1) - ga_concat(&ga, (char_u *)"..."); - ga_concat(&ga, (char_u *)arg_type); + ga_concat_len(&ga, (char_u *)"...", 3); + ga_concat_len(&ga, arg_type.string, arg_type.length); VIM_CLEAR(arg_free); } } @@ -2670,31 +2669,35 @@ type_name_func(type_T *type, char **tofree) for (i = 0; i < type->tt_argcount; ++i) { - char *arg_type; - int len; + string_T arg_type; if (type->tt_args == NULL) - arg_type = "[unknown]"; + { + arg_type.string = (char_u *)"[unknown]"; + arg_type.length = 9; + } else - arg_type = type_name(type->tt_args[i], &arg_free); + { + arg_type.string = (char_u *)type_name(type->tt_args[i], &arg_free); + arg_type.length = STRLEN(arg_type.string); + } if (i > 0) { STRCPY((char *)ga.ga_data + ga.ga_len, ", "); ga.ga_len += 2; } - len = (int)STRLEN(arg_type); - if (ga_grow(&ga, len + 8) == FAIL) + if (ga_grow(&ga, (int)arg_type.length + 8) == FAIL) goto failed; if (varargs && i == type->tt_argcount - 1) - ga_concat(&ga, (char_u *)"..."); + ga_concat_len(&ga, (char_u *)"...", 3); else if (i >= type->tt_min_argcount) *((char *)ga.ga_data + ga.ga_len++) = '?'; - ga_concat(&ga, (char_u *)arg_type); + ga_concat_len(&ga, arg_type.string, arg_type.length); VIM_CLEAR(arg_free); } if (type->tt_argcount < 0) // any number of arguments - ga_concat(&ga, (char_u *)"..."); + ga_concat_len(&ga, (char_u *)"...", 3); if (type->tt_member == &t_void) STRCPY((char *)ga.ga_data + ga.ga_len, ")");