On Wed, Sep 22, 2010 at 8:47 AM, Amos Jeffries <squ...@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.. > > ... > > >> 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. Changed to reflect this. MemBlob.mem can never be NULL; we could add a guard if the user passes a NULL char*, and treat it as a NOP. > 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. Can change to memmove; is there a performance hit? -- /kinkie