[squid-dev] [PATCH] Extend SBufContainerJoin to have prefix and suffix arguments

[Kinkie]

On Thu, Nov 3, 2016 at 10:55 PM, Alex Rousskov
<rousskov at measurement-factory.com> wrote:
> On 11/03/2016 03:19 PM, Kinkie wrote:
>> On Tue, Nov 1, 2016 at 8:47 PM, Alex Rousskov wrote:
>>> On 11/01/2016 02:02 PM, Kinkie wrote:
>>>>   the attached patch extends SBufContainerJoin to have prefix and
>>>> suffix arguments.
>
>>> I recommend reworking this by adding a dest parameter:
>>> SBuf &ContainerToSBuf(SBuf &dest, ...);
>
>
>> Can simply return the modified SBuf.
>
>>  SBuf
>> -SBufContainerJoin(const Container &items, const SBuf& separator)
>> +JoinContainerIntoSBuf(SBuf dest, const ContainerIterator &begin,
>
> This implementation does not avoid copies in a general case: When I
> already have an SBuf with some content, I have to feed my writeable SBuf
> to ContainerToSBuf() to avoid allocating and copying.
>
> AFAICT, the reasonable implementation options are:
>
>   1. Your old simpler implementation without "dest".
>   2. A more efficient implementation with writeable "dest".

attached v3 does this.

> The latest implementation has the complexity of #2 but lacks its
> efficiency. I do not think it is a good API.

I am not generally worried as it seems you are about copying SBufs as
long as the underlying storage is not reallocated needlessly. But it's
very simple to express one call in terms of the other, so I'm doing
just that. I am convinced that the end result of our discussion is
better than what we had started with, and I'm grateful for that.


-- 
    Francesco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sbufcontainerjoin-v3.patch
Type: application/octet-stream
Size: 3578 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20161104/ad1da0e0/attachment.obj>