On 1/08/2013 12:58 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:46 PM, Amos Jeffries <[email protected]> wrote:
On 1/08/2013 12:18 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov
<[email protected]> wrote:
On 07/30/2013 03:56 AM, Kinkie wrote:
lp:~squid/squid/stringng-cherrypick has the most uptodate code.
I did not finish the review before running out of time, but I found one
or two bugs in r12761 (in the code I have not looked at before). There
are also a few polishing issues:
Thanks.
typedef int32_t size_type;
I think we should make it unsigned. I agree with your earlier
observation that it is better to do that now rather than later or never.
Yes, it is agreed. It's however something I'd prefer to do as a
separate change, after everything else has been defined in order to
have a clear impact.
Yes and no. It will probably cause a lot of little changes all over the
classes involved. So doing it at a point of stability as a separate commit
to stringng is good but not a reason to
How about doing it after this round of reviews is done?
That should be a reasonable compromise.
void reserveSpace(size_type minSpace)
{reserveCapacity(length()+minSpace);}
As I mentioned before, this lacks overflow control if the sum of
length() and minSpace exceeds size_type maximum. Please add a check.
reserve* calls cow() which - if a resizing is needed - calls reAlloc
which throws SBufTooBigException if the requested size is too big.
Isn't that good enough?
Not if the math overflowed down to a smaller value before it even got passed
to reserveCapacity().
Ok. I'm going to check minSpace. maxSize+minSpace is definitely not
enough to overflow size_type
minSpace is controlled completely by the unknown caller code. It may be
UINT_MAX or something equally capable of overflowing when you add to it.
I think the right check for here is:
if (maxSize - length() < minSpace || minSpace < 0) abort().
The math being done on the two values we are confident about already and
ensuring that the leftover space is enough for the caller.
The extra 0 check disappearing when we move to unsigned of course.
Amos
