Fixed sdscat*() bug: read memory error

[iiol]

Example:

sds str;

str = sdsnew("Hi ");
str = sdscat(str, str);
printf("%s\n", str); // not "Hi Hi "

[UmanShahzad]

You can simplify this with memmove.

[cassepipe]

Wouldn't a call to memmove affect performance for big strings ? Is it not better to mention in the doc that sdscat should not take two same pointers ?

[UmanShahzad]

It depends; some implementations of memmove will check for an overlap first, and if there isn't one will delegate to memcpy.

Example of musl libc doing this: https://git.musl-libc.org/cgit/musl/tree/src/string/memmove.c#n15

[fractalb]

This is a non-issue. This is not the intended way of using sdscat function. It's defined with this prototype: sds sdscatlen(sds s, const void *t, size_t len) which promises not to modify the second argument t. It forbids the case where s == t. Maybe, add a comment about this limitation

[UmanShahzad]

Then how is the overlap case ever going to be supported?

Either brand new interfaces with the exact same code but with memcpy -> memmove (and there are many functions calling into this), thus significantly complicating the interface, or remove const and break backwards compatibility at the interface (warnings will be generated for users who were passing in const arguments into t).

The simplest solution is to just use memmove, which has practically no extra cost because it will still delegate to memcpy, and note in the comments this edge case.