It was a false positive, but it can't hurt to "fix" it.
Original warning:
CID 13685 (#1 of 1): Buffer not null terminated (BUFFER_SIZE)
6. buffer_size: Calling strncpy with a source string whose length (4 chars)
is greater than or equal to the size argument (4) will fail to
null-terminate b0p->b0_version.
Coverity detected a memory leak caused by not free'ing the value returned by
get_expr_line_src (basically vim_strsave(expr_line)). Replaced the copying
with direct manipulation of expr_line, since that also happens in other
parts of the codebase.
NOTE: I'm aware that this has different behaviour than vim_strnsave, namely
vim_strnsave always allocates `len` bytes, even if the string is shorter. I
don't see how that behaviour is helpful here though.
Also constified the arguments. The double casts for the `xstrdup` are ugly
but `vim_strsave` doesn't take `const` arguments for now so I couldn't keep
that.
- The data member of String's can now be passed directly to functions
expecting C strings, as we now guarantee that they are NUL-terminated.
This obviates the need to use xstrndup and free, simplifying code and
enhancing performance.
- Use xmemdupz instead of xstrndup for converting String's into C strings.
It's faster because it doesn't calculate strlen(string.data) (which is
unnecesary as that information is already provided in string.size anyway).
- Use cstr_to_string to convert from C strings to String, it is both shorter
and faster than the usual strlen/xstrndup combo, which calls strlen twice.
cstr_to_string internally calls strlen and then xmemdupz.
I believe we can now mostly assume that all encountered String's data
members are safe to pass into functions that accept C strings. That should
simplify interop with C string code.
It's a 1-byte loss of memory but it allows us to skip copying and
NULL-terminating strings when interacting with vim functions that accept C
strings. This lowers the pressure on the allocator and saves lines of code
(no more dup/free pairs).
mb_string2cells was always called like mb_string2cells(..., -1) so that was
the only codepath that was tested. @tarruda was the first to try to input an
actual length, after which valgrind detected that funny business was going
on.
It's not even possible to do the right thing with the current text codec
infrastructure: they all assume to be working with C strings. Meaning that
if there is no NUL-terminator, they will happily keep on reading past the
end of Pascal strings. Ergo, passing the length parameter is moot. The
condition in the for-loop was wrong as well (but that's no longer relevant).
Also change the return value to size_t, by analogy with strlen.
ref:
677d30d796
Most of these functions don't modify their strings, let's make the contract
a bit clearer. In some cases I've tried to get rid of C89-style variable
declarations at the start of functions, now that I was there.
