On 27/04/17 10:58, Alex Rousskov wrote:
Hello,
I accidentally discovered that our xstrndup() documentation lies and
its implementation is dangerously nothing like the standard one. The
attached patch fixes documentation and three callers. Please see the
patch preamble for details, including lack of ESI testing.
The attached patch is NOT the right long-term solution because folks
will, naturally, continue to use xstrndup(SQUID) as if it is strndup(3),
leading to new caller bugs.
Unfortunately, just fixing xstrndup() implementation and callers is not
enough! If we fix xstrndup() implementation (and existing callers) in
v5, then any new caller code ported to an older Squid version may break
in subtle ways (because that old Squid version assumes 0-termination and
requires "+1s" to work correctly). No human will be able to spot such
porting errors reliably.
Thus, it looks like we have to rename xstrndup(), forcing porting humans
to notice the change in the API and adjust the callers. I can suggest
xstrndupe() or bufDupe() as the new name.
Would anybody volunteer to fix this for the long-term?
In the long term the code using it will be refactored use SBuf or
std::string anyway. I think it would be best if anyone taking this
request up try to do that migration early rather than trying to fix what
is just a portability hack - and for a convenience function at that.
If that is too hard, making callers do the allocate bit themselves and
xstrncpy() instead should be a good medium-term workaround.
Thank you,
Alex.
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() ?
+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.
Amos
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
