Re: [PATCH v2] string.c: remove use of strcat() after strlen()

[Oliver Kiddle <opk@zsh.org>]

On 31 Dec, James wrote:
> Since we are already calling strlen() beforehand to know the size of
> allocation, we can just memcpy() from BASE + BASE_LEN and avoid another strlen
> () call in strcat(). We should prefer memcpy() because we always know the
> length and most libc implementations provide an assembly implementation of
> memcpy but maybe not strcpy(). Even if it is implemented in assembly, memcpy()
> is likely to be faster than strcpy() since as the loop condition strcpy() needs
> to check for zeros in SRC, whereas memcpy() can just decrement the size.
>
> I'm using a MEMPCPY macro because I don't know how zsh handles using GNU/POSIX
> extensions.

Many OS-specific extensions are used. We'd normally add a test in
configure.ac and then check for that so the macro could be wrapped in
#ifndef HAVE_MEMPCPY
And such macros may preserve the original lowercase which can be less
ugly. Calling the actual system mempcpy may be more efficient than the
macro because it probably already has a pointer to the end. FreeBSD
also has mempcpy().

If it is meant as an optimisation it could be worth verifying that it
has an effect. I don't especially find the modified code any easier to
read. I find it quicker and easier to scan when simple nested calls like
strlen() are not first taken out and assigned to a variable. If it does
make things faster, there may be other places in the code where appstr()
could have been used but strcat/realloc were used directly.

Oliver