Now I see.
I have worked on this too mich - I keep seeing what I believe it is
instead of what it actually is.
Fixed in both rawSpace and reserveSpace.
On Wed, Jul 31, 2013 at 7:12 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/31/2013 11:11 AM, Alex Rousskov wrote:
On
Another small bug in that code is that arguments or even fmt might
actually end with a valid \0 byte that should be appended and counted
(not a c-string terminating byte).
This check should be done once, with controlled fmt and arguments so
that we can detect problems reliably. You can do
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov
rouss...@measurement-factory.com 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
On 1/08/2013 12:18 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov
rouss...@measurement-factory.com 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
On Wed, Jul 31, 2013 at 2:46 PM, Amos Jeffries squ...@treenet.co.nz wrote:
On 1/08/2013 12:18 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/30/2013 03:56 AM, Kinkie wrote:
lp:~squid/squid/stringng-cherrypick has the most
On 1/08/2013 12:58 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:46 PM, Amos Jeffries squ...@treenet.co.nz wrote:
On 1/08/2013 12:18 a.m., Kinkie wrote:
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/30/2013 03:56 AM, Kinkie wrote:
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
On 07/31/2013 06:18 AM, Kinkie wrote:
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,
On 07/31/2013 10:12 AM, Kinkie wrote:
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
On 07/31/2013 11:11 AM, Alex Rousskov wrote:
On 07/31/2013 10:12 AM, Kinkie wrote:
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
On 07/31/2013 06:18 AM, Kinkie wrote:
On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov wrote:
On 07/30/2013 03:56 AM, Kinkie wrote:
len_ += sz;
// TODO: this does NOT belong here, but to class-init or autoconf
/* on Linux and FreeBSD, '\0' is not counted in return value */
/*
Ok, it's now done, including documentation.
lp:~squid/squid/stringng-cherrypick has the most uptodate code.
On Mon, Jul 29, 2013 at 11:55 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/29/2013 01:38 PM, Kinkie wrote:
So to implement this I should:
- rename current
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:
typedef
On Sun, Jul 28, 2013 at 1:03 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 02:57 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 10:32 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 01:03 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex
On 07/29/2013 03:02 AM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b. Reserve buffer space. Ensure exclusive buffer ownership.
2. Reserve N space bytes for the caller to append to. No guarantees
On Mon, Jul 29, 2013 at 5:32 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/29/2013 03:02 AM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b. Reserve buffer space. Ensure exclusive
On 07/29/2013 01:38 PM, Kinkie wrote:
So to implement this I should:
- rename current reserveSpace to rawSpace, add returning the pointer
as in current rawSpace.
- reimplement reserveSpace as a convenience calling reserveCapacity
- adjust clients of reserveSpace to use rawSpace instead.
On 27/07/2013 11:38 a.m., Alex Rousskov wrote:
On 07/26/2013 03:27 PM, Kinkie wrote:
Resending due to MUA insisting on using HTML email..
On Thu, Jul 18, 2013 at 1:06 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/04/2013 11:51 PM, Kinkie wrote:
void
On 28/07/2013 12:00 a.m., Kinkie wrote:
Here is the use case I am thinking about:
sbuf.reserveSpace(ioSize);
bytesRead = read(sbuf.rawSpace(), ioSize);
sbuf.forceSize(sbuf.size() + bytesRead);
This explains it all, we're thinking about two different use cases.
The other use case
On 07/27/2013 05:35 AM, Amos Jeffries wrote:
* ensure it is 0-safe; the space starts with 0-terminator
Please no. We have enough bugs with implicit zero terminator being
forced into buffers that do not need one, screwing up buffer size
management and related optimizations/assertions. I suspects
On Sat, Jul 27, 2013 at 5:57 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 05:35 AM, Amos Jeffries wrote:
* ensure it is 0-safe; the space starts with 0-terminator
Please no. We have enough bugs with implicit zero terminator being
forced into buffers that do not
On 07/27/2013 06:00 AM, Kinkie wrote:
Here is the use case I am thinking about:
sbuf.reserveSpace(ioSize);
bytesRead = read(sbuf.rawSpace(), ioSize);
sbuf.forceSize(sbuf.size() + bytesRead);
The other use case is:
sbuf.reserveCapacity(newCapacity);
On Sat, Jul 27, 2013 at 6:39 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 06:00 AM, Kinkie wrote:
Here is the use case I am thinking about:
sbuf.reserveSpace(ioSize);
bytesRead = read(sbuf.rawSpace(), ioSize);
sbuf.forceSize(sbuf.size() + bytesRead);
On 07/27/2013 12:00 PM, Kinkie wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b. Reserve buffer space. Ensure exclusive buffer ownership.
2. Reserve N space bytes for the caller to append to. No guarantees
regarding buffer ownership are provided.
I like
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 12:00 PM, Kinkie wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b. Reserve buffer space. Ensure exclusive buffer ownership.
2. Reserve N space bytes for
On 07/27/2013 01:03 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
On 07/27/2013 12:00 PM, Kinkie wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b. Reserve buffer space. Ensure exclusive buffer ownership.
2. Reserve N space bytes for
On Sat, Jul 27, 2013 at 10:32 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 01:03 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
On 07/27/2013 12:00 PM, Kinkie wrote:
1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
1b.
On 07/27/2013 02:57 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 10:32 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/27/2013 01:03 PM, Kinkie wrote:
On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
On 07/27/2013 12:00 PM, Kinkie wrote:
1a. Reserve total buffer capacity.
On 07/26/2013 03:27 PM, Kinkie wrote:
Resending due to MUA insisting on using HTML email..
On Thu, Jul 18, 2013 at 1:06 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 07/04/2013 11:51 PM, Kinkie wrote:
void
SBuf::reserveCapacity(size_type minCapacity)
{
Must(0 =
On 07/04/2013 11:51 PM, Kinkie wrote:
void
SBuf::reserveCapacity(size_type minCapacity)
{
Must(0 = minCapacity minCapacity = maxSize);
reserveSpace(minCapacity-length());
}
This does not look right. For example, if minCapacity is smaller than
length(), we should do
On 9/07/2013 8:16 p.m., Kinkie wrote:
Also, I suggest splitting this into two methods, one with a required
(first?) SBufCaseSensitive parameter and one without it. This will allow
callers to specify n without specifying isCaseSensitive and vice versa.
The shorter, inlined method will simply call
That comment is probably stale because canAppend() is currently not
related to ownership of the buffer: It will happily return true for
shared buffers. AFAICT, reserveSpace() should be implemented using
something like this:
Must(0 = minSpace minSpace = maxSize - length());
Snipped out the Done bits.
On 5/07/2013 5:51 p.m., Kinkie wrote:
On Tue, Apr 16, 2013 at 12:49 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
Hi Kinkie,
Here is my review of
https://code.launchpad.net/~squid/squid/stringng-cherrypick r12753. I
found approximately three bugs. I
On Tue, Apr 16, 2013 at 12:49 AM, Alex Rousskov
rouss...@measurement-factory.com wrote:
Hi Kinkie,
Here is my review of
https://code.launchpad.net/~squid/squid/stringng-cherrypick r12753. I
found approximately three bugs. I am also concerned about size_type
type, after starting to use
Hi Kinkie,
Here is my review of
https://code.launchpad.net/~squid/squid/stringng-cherrypick r12753. I
found approximately three bugs. I am also concerned about size_type
type, after starting to use MemBlob which has the same problem. The rest
is polishing.
fgrep npos SBuf.cc
...
if
-- This is not a review, but I wanted to clarify or emphasize some of
the points in Amos' review. Thank you Amos for working on this!
On 03/30/2013 02:32 AM, Amos Jeffries wrote:
in src/SBuf.cc:
* SBuf::chop() - the TODO optimization is not possible. We cannot be
completely sure we are teh
Hi, I forgot to mention another couple of changes:
- I added an explicit copy-constructor to import std::string
- I relaxed the behavior of the chop() method for out-of-bounds cases:
instead of throwing, it'll now ignore the part of the specified limits
that is out of bounds.
On Mon, Apr 1, 2013
On 04/01/2013 11:43 AM, Kinkie wrote:
Attached is a bundle containing the changes from this review.
Will you commit that to your stringng branch on lp? Or should I patch
that branch before reviewing/testing instead?
Thank you,
Alex.
On 2/04/2013 6:57 a.m., Alex Rousskov wrote:
-- This is not a review, but I wanted to clarify or emphasize some of
the points in Amos' review. Thank you Amos for working on this!
On 03/30/2013 02:32 AM, Amos Jeffries wrote:
in src/SBuf.cc:
* SBuf::chop() - the TODO optimization is not
On 30/03/2013 7:05 a.m., Kinkie wrote:
Hi,
it was easier than expected. Here's a merge bundle; it intentionally
forgets commit history, it can remain in the branch. This bundle
passes a full testsuite build on my kubuntu quantal.
It contains only what was audited so far:
- SBuf
-
On 29/03/2013 11:46 p.m., Kinkie wrote:
Hi all,
it's been a while since the last StringNG merge checkpoint , and
several improvements were made in the meantime.
I have briefly reviewed the previous requests for changes, I don't
think there's any outstanding change requests; the new unit
On 03/29/2013 04:46 AM, Kinkie wrote:
it's been a while since the last StringNG merge checkpoint , and
several improvements were made in the meantime.
I have briefly reviewed the previous requests for changes, I don't
think there's any outstanding change requests; the new unit testing
Ok, it's agreed then, I'll cherrypick.
File-wise it's not hard. I'm a bit worried about automake , but I'll manage.
On Fri, Mar 29, 2013 at 4:14 PM, Alex Rousskov
rouss...@measurement-factory.com wrote:
On 03/29/2013 04:46 AM, Kinkie wrote:
it's been a while since the last StringNG merge
43 matches
Mail list logo
