[squid-dev] [PATCH] xstrndup has to go

[Alex Rousskov]

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?


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.

P.P.S. I did not find any callers that are likely to crash because of
our incorrect xstrndup() implementation, although some callers will
crash if String is changed to use unterminated raw buffers again:

   xstrndup(value.rawBuf(), value.size() + 1);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: xstrndup-is-not-t3.patch
Type: text/x-diff
Size: 7214 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170426/6f62cc50/attachment.patch>