Re: SBuf review at r9370

2009-03-05 Thread Kinkie
On Wed, Mar 4, 2009 at 9:45 PM, Alex Rousskov wrote: > On 03/04/2009 06:09 AM, Kinkie wrote: >> Who'd be in charge of managing the passed memory block? >> I see two choices: >> - the caller is in charge >>   then absorb becomes an alias of assign() and has no reason to exist >> except create confu

Re: SBuf review at r9370

2009-03-04 Thread Amos Jeffries
* Remove isImported. Copy and then free the buffer when importing instead. Same motivation as in the isLiteral comment above. >>> This too has a IMO a very practical use: it allows us an easy path to >>> get into the SBuf world i/o buffer which have been created by the i/o >>> layer.

Re: SBuf review at r9370

2009-03-04 Thread Alex Rousskov
On 03/04/2009 06:09 AM, Kinkie wrote: > Who'd be in charge of managing the passed memory block? > I see two choices: > - the caller is in charge > then absorb becomes an alias of assign() and has no reason to exist > except create confusion > - absorb frees > this would make the behaviour more

Re: SBuf review at r9370

2009-03-04 Thread Kinkie
>>> * Remove isImported. Copy and then free the buffer when importing >>> instead. Same motivation as in the isLiteral comment above. >>> >> This too has a IMO a very practical use: it allows us an easy path to >> get into the SBuf world i/o buffer which have been created by the i/o >> layer. If yo

Re: SBuf review at r9370

2009-03-02 Thread Alex Rousskov
On 03/02/2009 10:36 AM, Kinkie wrote: > BUT MemPools are initialized AFTER global variables, and thus a global > definition such as: > SBuf foo("some initializer"); > causes the SBuf to grow() > which calls memAllocString > which tries to locate a pool, and fails because the pools are not yet ready

Re: SBuf review at r9370

2009-03-02 Thread Kinkie
On Thu, Feb 26, 2009 at 7:08 PM, Alex Rousskov wrote: > On 02/26/2009 10:24 AM, Kinkie wrote: >> On Thu, Feb 26, 2009 at 1:46 AM, Alex Rousskov >> wrote: >> >>> http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. >>> >>> * Remove isLiteral and the corresponding code/logic. We might add t

Re: SBuf review at r9370

2009-02-27 Thread Amos Jeffries
Kinkie wrote: Um, IMO its better to be liberal with length() and very,very conservative with len_. I think I reached a sensible compromise, however subjective. I ask you not to be too strict now, but to feel free to change it after the branch lands into trunk. Dumping stuff and pointer maths

Re: SBuf review at r9370

2009-02-27 Thread Kinkie
> Um, IMO its better to be liberal with length() and very,very conservative > with len_. I think I reached a sensible compromise, however subjective. I ask you not to be too strict now, but to feel free to change it after the branch lands into trunk. > Dumping stuff and pointer maths particularly

Re: SBuf review at r9370

2009-02-26 Thread Alex Rousskov
On 02/26/2009 08:46 PM, Amos Jeffries wrote: > Kinkie wrote: >>> * Please use .length() everywhere you can. Do not mix .len_ and >>> .length(). >> Ok, checked. >> I've left len_ where it's being assigned to another object's len_ or >> in very low-level stuff (e.g. object dumping, stuff doing pointe

Re: SBuf review at r9370

2009-02-26 Thread Amos Jeffries
Kinkie wrote: On Thu, Feb 26, 2009 at 1:46 AM, Alex Rousskov wrote: Hi Kinkie, Here are a few corrections for http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. * Remove isLiteral and the corresponding code/logic. We might add this very minor performance optimization as a non-pub

Re: SBuf review at r9370

2009-02-26 Thread Kinkie
On Thu, Feb 26, 2009 at 1:46 AM, Alex Rousskov wrote: > Hi Kinkie, > >    Here are a few corrections for > http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. > > * Remove isLiteral and the corresponding code/logic. We might add this > very minor performance optimization as a non-public B

Re: SBuf review at r9370

2009-02-26 Thread Alex Rousskov
On 02/26/2009 10:24 AM, Kinkie wrote: > On Thu, Feb 26, 2009 at 1:46 AM, Alex Rousskov > wrote: > >> http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. >> >> * Remove isLiteral and the corresponding code/logic. We might add this >> very minor performance optimization as a non-public B

Re: SBuf review at r9370

2009-02-25 Thread Alex Rousskov
On 02/25/2009 07:37 PM, Amos Jeffries wrote: > Alex Rousskov wrote: >> * I wonder if SBuf::grow() should be moved to MemBlob that may >> (eventually) have a low-level knowledge on how to do that efficiently. >> What do you and others think? > > As long as MemBlob::grow() returns a pointer/reference

Re: SBuf review at r9370

2009-02-25 Thread Amos Jeffries
Alex Rousskov wrote: Hi Kinkie, Here are a few corrections for http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. * Remove isLiteral and the corresponding code/logic. We might add this very minor performance optimization as a non-public Blob member once the code is known to work w

SBuf review at r9370

2009-02-25 Thread Alex Rousskov
Hi Kinkie, Here are a few corrections for http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370. * Remove isLiteral and the corresponding code/logic. We might add this very minor performance optimization as a non-public Blob member once the code is known to work well. For now, the code

Re: Sbuf review at r9331

2009-02-24 Thread Kinkie
>> - check that the function names in debugs() statements match their >> actual name (there's been quite a lot of shuffling) > > Please use debugs(..., HERE << "blah") for that sort of thing at levels >1. > I've also just added MYNAME for prettier but identical use on the top two > levels. Now don

Re: Sbuf review at r9331

2009-02-23 Thread Amos Jeffries
> On Mon, Feb 23, 2009 at 11:58 PM, Alex Rousskov > wrote: >> On 02/23/2009 10:52 AM, Kinkie wrote: >> * Declare when needed, for example inside for(): Â size_type j; Â for (j=0;j>> >>> Do you mean that this should read >>> for (size_type j=0;j>> ? >> >> Yes, I do. IIRC, there are

Re: Sbuf review at r9331

2009-02-23 Thread Kinkie
On Mon, Feb 23, 2009 at 11:58 PM, Alex Rousskov wrote: > On 02/23/2009 10:52 AM, Kinkie wrote: > >>> * Declare when needed, for example inside for(): >>>   size_type j; >>>   for (j=0;j> >> Do you mean that this should read >> for (size_type j=0;j> ? > > Yes, I do. IIRC, there are many other examp

Re: Sbuf review at r9331

2009-02-23 Thread Alex Rousskov
On 02/23/2009 10:52 AM, Kinkie wrote: >> * Declare when needed, for example inside for(): >> size_type j; >> for (j=0;j > Do you mean that this should read > for (size_type j=0;j ? Yes, I do. IIRC, there are many other examples where variables are declared too early. This is a minor flaw, wi

Re: Sbuf review at r9331

2009-02-23 Thread Kinkie
Further improvements: > * SBufStore(const SBuf &S); > > SBufStore must not know about SBuf. Done. It's now a first-class citizen, without any SBuf knowledge. In the process I've removed some methods and migrated others to char*-based. > * Why is Sbuf MemPooled?! We are not supposed to allocate S

Re: Sbuf review at r9331

2009-02-20 Thread Alex Rousskov
On 02/20/2009 12:50 PM, Kinkie wrote: >> 2) Confusion regarding pooling of SBuf objects is suspected (discussed >> in a few bullets below). >> > Given your explanation it is now a certainty. > On the other hand, SBufStore objects would probably benefit from it, > wouldn't they? > Yes, they

Re: Sbuf review at r9331

2009-02-20 Thread Kinkie
On Fri, Feb 20, 2009 at 5:56 PM, Alex Rousskov wrote: > On 02/20/2009 05:16 AM, Kinkie wrote: >> On Sun, Feb 1, 2009 at 9:08 PM, Alex Rousskov >> wrote: >>> Here are a few Buffer comments based on r9331 of lp:~kinkie/squid/stringng > > Executive summary: Many problems fixed, thank you. We are m

Re: Sbuf review at r9331

2009-02-20 Thread Alex Rousskov
On 02/20/2009 05:16 AM, Kinkie wrote: > On Sun, Feb 1, 2009 at 9:08 PM, Alex Rousskov > wrote: >> Here are a few Buffer comments based on r9331 of lp:~kinkie/squid/stringng Executive summary: Many problems fixed, thank you. We are making progress. Three known big issues remain: 1) SBufStore sh

Re: Sbuf review at r9331

2009-02-20 Thread Kinkie
On Sun, Feb 1, 2009 at 9:08 PM, Alex Rousskov wrote: > Hi Kinkie, > > Here are a few Buffer comments based on r9331 of lp:~kinkie/squid/stringng > The review is incomplete as there is too much in the way to do a full > high-level and low-level review. Some of the comments below will help to > c

Sbuf review at r9331

2009-02-01 Thread Alex Rousskov
Hi Kinkie, Here are a few Buffer comments based on r9331 of lp:~kinkie/squid/stringng The review is incomplete as there is too much in the way to do a full high-level and low-level review. Some of the comments below will help to clean up the way for a full review. The common theme is: -

Re: Request for new round of SBuf review

2008-12-11 Thread Alex Rousskov
On Sun, 2008-12-07 at 21:26 +0100, Kinkie wrote: > Not much new code yet, but as a result of our discussion there's a > small adjustmdent in strategy, which in the short term has two parts: > the code I posted is valid and I still wish it to be reviewed. SBuf 's > purpose changes as it'll aim at c

Re: Request for new round of SBuf review

2008-12-07 Thread Kinkie
Hi all, Not much new code yet, but as a result of our discussion there's a small adjustmdent in strategy, which in the short term has two parts: the code I posted is valid and I still wish it to be reviewed. SBuf 's purpose changes as it'll aim at catering the need to handle blobs I've set off wri

Re: Request for new round of SBuf review

2008-12-06 Thread Adrian Chadd
Howdy, As most of you aren't aware, Kinkie, alex and I had a bit of a discussion about this on IRC rather than on the mailing list, so there's probably some other stuff which should be posted here. Kinkie, are you able to post some updated code + docs after our discussion? My main suggestion to

Request for new round of SBuf review

2008-12-04 Thread Kinkie
Hi all, I feel that SBuf may just be complete enough to be considered a viable replacement for SquidString, as a first step towards integration. I'd appreciate anyone's help in giving it a check to gather feedback and suggestions. Doxygen documentation for the relevant classes is available at h

Re: SBuf review

2008-09-19 Thread Alex Rousskov
On Fri, 2008-09-19 at 13:08 +0800, Adrian Chadd wrote: > I'll say it again - ignore MemBuf. Ignore MemBuf for now. Leave it as > a NUL-terminated dynamic buffer with some printf append like > semantics. > > When you've implemented a non-NUL-terminated ref-counted memory region > implementation an

Re: SBuf review

2008-09-18 Thread Amos Jeffries
Adrian Chadd wrote: 2008/9/19 Amos Jeffries <[EMAIL PROTECTED]>: I kind of fuzzily disagree, the point of this is to replace MemBuf + String with SBuf. Not implement both again independently duplicating stuff. I'll say it again - ignore MemBuf. Ignore MemBuf for now. Leave it as a NUL-termina

Re: SBuf review

2008-09-18 Thread Adrian Chadd
2008/9/19 Amos Jeffries <[EMAIL PROTECTED]>: > I kind of fuzzily disagree, the point of this is to replace MemBuf + String > with SBuf. Not implement both again independently duplicating stuff. I'll say it again - ignore MemBuf. Ignore MemBuf for now. Leave it as a NUL-terminated dynamic buffer w

Re: SBuf review

2008-09-18 Thread Alex Rousskov
On Fri, 2008-09-19 at 14:36 +1200, Amos Jeffries wrote: > >> *B3* If a class does not have both dump() and print() methods, let's > >> call the only printing method print(). This will make it easier to > >> manipulate printable objects for debugging. > > > > The convention I followed is: dump()

Re: SBuf review

2008-09-18 Thread Amos Jeffries
Kinkie wrote: On Thu, Sep 18, 2008 at 7:26 PM, Alex Rousskov <[EMAIL PROTECTED]> wrote: On Thu, 2008-09-18 at 14:10 +0200, Kinkie wrote: The feature branch is at https://code.launchpad.net/~kinkie/squid/stringng Here is another round of comments, based on the current SBuf.h in the above repos

Re: SBuf review

2008-09-18 Thread Alex Rousskov
On Thu, 2008-09-18 at 22:14 +0200, Kinkie wrote: > > *B1* I still think that string-manipulation interfaces should be moved > > into a String class. > > I could agree to that if I could see an equally efficient way to implement > this. > I've called the class SBuf because I see it as being a bit

Re: SBuf review

2008-09-18 Thread Kinkie
On Thu, Sep 18, 2008 at 7:26 PM, Alex Rousskov <[EMAIL PROTECTED]> wrote: > On Thu, 2008-09-18 at 14:10 +0200, Kinkie wrote: > >> The feature branch is at https://code.launchpad.net/~kinkie/squid/stringng > > Here is another round of comments, based on the current SBuf.h in the > above repository.

SBuf review

2008-09-18 Thread Alex Rousskov
On Thu, 2008-09-18 at 14:10 +0200, Kinkie wrote: > The feature branch is at https://code.launchpad.net/~kinkie/squid/stringng Here is another round of comments, based on the current SBuf.h in the above repository. I have marked my comments with these prefixes: *B1*: a blocking issue. I do not re