Re: StringNg review: MemBlob

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);

...

> but we should not mention "string" in MemBlob.cci.
> MemBlob does not (or should not) know what its users are. There may be a
> better way to express the same thought. Consider:
>
> \none memcpy() is safe because we copy to an unused area

*that* is better. There is still no guarantee it wont overflow on the
destination. memcpy() makes no mention about when happens when one of
the pointers is NULL, but experience shows a lot of segfaults.

memcpy() docs state "always copies exactly num bytes" and that the
buffers should not overlap.

memmove() is the safe one which should be used if there is any doubt
about overlapping memory. But even that can't validate against overflows.

>
> The note belongs to the append() implementation, not its description.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2