Skip to content

Pass string length to list_append_string() where it is known#19491

Closed
basilisk0315 wants to merge 5 commits intovim:masterfrom
basilisk0315:master
Closed

Pass string length to list_append_string() where it is known#19491
basilisk0315 wants to merge 5 commits intovim:masterfrom
basilisk0315:master

Conversation

@basilisk0315
Copy link

@basilisk0315 basilisk0315 commented Feb 23, 2026

This PR refactors some source files to pass string length to function list_append_string() where it is known or can be calculated simply.

In addition:
In evalfunc.c:
refactor function block_def2str() to accept a string_T to return the resulting string and to drop local variables.
refactor function f_getregion() to restructure the logic within the for loop and to use a string_T to store string akt.
In register.c:
refactor function get_reg_contents() to exit the first for loop if the call to list_append_string() fails which means we can remove local variable error.
In strings.c:
refactor function convert_string() to accept a string_T and another string_T to return the resulting string.
refactor function string_from_blob() to accept a string_T to return the resulting string.
refactor function append_converted_string_to_list() to accept argument converted as a string_T.
refactor function append_validated_line_to_list() to accept argument line as a string_T.

In evalbuffer.c:
refactor function get_buffer_lines() to call vim_strnsave() instead of vim_strsave().

Cheers
John

@basilisk0315
Copy link
Author

basilisk0315 commented Feb 24, 2026

I am seeing build failures like so:

In file included from /usr/include/string.h:548,
                 from os_unix.h:461,
                 from vim.h:284,
                 from strings.c:15:
In function ‘strncpy’,
    inlined from ‘vim_strnsave’ at strings.c:48:5,
    inlined from ‘string_from_blob’ at strings.c:1283:16,
    inlined from ‘f_blob2str’ at strings.c:1514:10:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ destination unchanged after copying no bytes [-Werror=stringop-truncation]
   95 |   return __builtin___strncpy_chk (__dest, __src, __len,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   96 |                                   __glibc_objsize (__dest));
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~

It seems that the compiler is tripping up on this construct (line 1283 in strings.c):

	ret->string = vim_strnsave((char_u *)"", 0);

Strangely, this construct is used elsewhere (see line 643 in file cmdhist.c, line 4952 in file ex_getln.c and line 1409 in file userfunc.c) which does not trigger the same error. In addition, the build times seem to be excessive long.

Weird.
John

@chrisbra
Copy link
Member

can you make this a ret->string = vim_strsave((char_u *)""); instead and see if this pleases the compiler?

@basilisk0315
Copy link
Author

Done. Let's see how it likes it.

@chrisbra
Copy link
Member

seems to be fine now

@chrisbra chrisbra closed this in 455d62e Feb 26, 2026
@chrisbra
Copy link
Member

thanks!

zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Feb 26, 2026
Problem:  Inefficient use of list_append_string()
Solution: Pass string length to list_append_string() where it is known
          (John Marriott).

closes: vim/vim#19491

vim/vim@455d62e

Co-authored-by: John Marriott <[email protected]>
zeertzjq added a commit to neovim/neovim that referenced this pull request Feb 26, 2026
…38083)

Problem:  Inefficient use of list_append_string()
Solution: Pass string length to list_append_string() where it is known
          (John Marriott).

closes: vim/vim#19491

vim/vim@455d62e

N/A patches:
vim-patch:9.2.0063: memory leak in type_name_list_or_dict()
vim-patch:9.2.0065: memory leak in invoke_sync_listeners()
vim-patch:9.2.0066: memory leak in build_drop_cmd()
vim-patch:9.2.0067: memory leak in dict_extend_func()

Co-authored-by: John Marriott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants