On Mon, Oct 18, 2010 at 7:56 PM, Alex Rousskov <[email protected]> wrote: > Hi Kinkie, > > Attached is am untested patch with some final polishing changes for > MemBlob, based on lp:~kinkie/squid/stringng revision 9517. I think the > MemBlob class should be committed to trunk after these changes are tested > and polished (I may have missed a few minor problems when moving or renaming > stuff). The rest of the branch code would need to be synced as well.
Polished and build-tested. Unless someone objects, I'm going to merge MemBlob to trunk this evening (GMT). It lacks unit-tests (SBuf's unit tests provide them indirectly). > > The patch preamble documents the major changes. > > While working on this, I realized that the "MemBlob::size" member is only an > approximation, and that MemBlob class is probably not usable for efficient > message body pipelining (BodyPipe and friends). We will probably change > MemBlob API later to accommodate users other than strings, but that can > wait. I adjusted the append call to remove the use of pointers which may > make future changes less disruptive. Fixed SBuf accordingly. > Once MemBlob is committed, please consider re-integrating it with memory > pools. This is the biggest remaining XXX, IMO. I've checked and confirmed why MemPools integration was disabled: it's due to an inherent asymmetry in memAllocString/memFreeString: if mempools are not initialized, memAllocString will happily allocate the string using xcalloc. When the time comes to deallocate such strings, it will fail to find a pool to dealloc from and fail an assertion. The only way I can think of to cleanly resolve this is quite disruptive, as it means making MemPools a proper singleton obect to make sure they are initialized before being used: it's a complex project on its own which I'd list as a dependency before integrating MemPools back in. IMVHO body pipelining should go through SBuf, not MemBlob, but that's for body pipelining to work out. -- /kinkie
