Re: StringNg review: MemBlob

On 09/22/2010 03:44 AM, Kinkie wrote:
> On Wed, Sep 22, 2010 at 8:47 AM, Amos Jeffries<squid3_at_treenet.co.nz> wrote:
>> On 22/09/10 16:31, Alex Rousskov wrote:
>>>
>>> On 09/20/2010 08:18 PM, Alex Rousskov wrote:
>>>>
>>>> On 09/03/2010 09:21 AM, Kinkie wrote:
>>>>
>>>>> You'll find the branch at lp:~kinkie/squid/stringng
>>>
>>> ...
>>>>
>>>> Comments for MemBlob.cci r9472:
>>>
>>> Found one more:
>>>
>>>> * \note memcpy is used as the copying function. This is safe,
>>>> * because it is guarranteed by design that the copied-to
>>>> * memory area is only owned by the appended-to string,
>>>> * and thus doesn't overlap with the appended-from area
>>>> */
>>>> void
>>>> MemBlob::append(const char *S, const MemBlob::size_type Ssize)
>>>> {
>>>> memcpy(mem+bufUsed,S,Ssize);
>>>> bufUsed+=Ssize;
>>>> ++stats.append;
>>>> }
>>>
>>> The \note is correct,
>>
>> No. As you pointed out to me earlier with SBuf the S here may be the result
>> of:
>> a.clear(); a.append(a.mem, 1);
>
> Using SBuf's raw memory access functions is dangerous, and this is
> well documented.
> It gives users rope, can't be blamed for how they use it..

This has little to do with raw memory access, actually.

If a is SBuf and a.mem is a.buf(), then

  a.clear(); a.append(a.buf(), 1);

is wrong not because some areas overlap (they actually do not, see my
earlier response) but because a.size() is less than 1 after a.clear()
and so the caller should not be using "1" as the size-to-append
parameter. In other words, there is a bug, but the bug is not in the
append(), but in the caller code.

I just realized that SBuf uses length() instead of standard size(). I
hope you got the idea though. :-(

Alex.
Received on Wed Sep 22 2010 - 18:08:29 MDT