On 04/29/2017 02:33 AM, Amos Jeffries wrote: > If that is too hard, making callers do the allocate bit themselves and > xstrncpy() instead should be a good medium-term workaround.
In general, it is impossible to correctly replace strndup() with a simple allocate and strncpy() pair. I bet this impossibility is why strndup() exists! strndup() is not a convenience function, but an irreplaceable tool in a performance toolbox. Why is it impossible in general? Because you do not know how many bytes to allocate (that number may be much smaller than n) and you cannot use strlen() to find out because the array may not be 0-terminated. You need strnlen(), which is usually not available where strndup() is not. >> P.S. The OutOfBoundsException use case code fixed by this patch needs a >> different fix (the one that does not allocate memory twice): >> >> xstrndup(explanatoryText.rawContent(), explanatoryText.length()); >> >> but we cannot do the above until xstrndup() implementation is fixed. > Not even by moving that caller to xmalloc() for itself and use xstrncpy() ? xmalloc()+xstrncpy() is not "doing the above" it is doing something else. I only claimed that we "cannot do the above" (literally). There are other ways to achieve the same result, of course. None of those other ways that I can think of (in the current code context) are better than the proposed double-copy IMO. Sure, it is possible to do xmalloc()+memcpy() in this context instead of xstrndup(), but, in this context, that would be arguably worse than copying twice because it would replace a clear high-level code with risky low-level manipulations on an error handling path. If strdup(buf.c_str()) pattern appears often, we should add SBuf::c_str_dup() that does xmalloc()+memcpy(). > +1 if you update the patch to do the above. +0 otherwise - ie. go ahead > with commit if you want, but I'd rather see the fixed caller(s) fixed to > use a more long-ish term fix. Understood. We disagree regarding the "more long-ish" fix desirability. Thank you, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
