On Tue, Feb 4, 2014 at 2:17 AM, Alex Rousskov <rouss...@measurement-factory.com> wrote: > On 02/02/2014 02:42 PM, Amos Jeffries wrote: > >> in src/HttpHdrRange.cc >> >> * This pattern happens several times throughout this patch: >> - while (!specs.empty()) >> - delete specs.pop_back(); >> + while (!specs.empty()) { >> + delete specs.back(); >> + specs.pop_back(); >> + } >> >> ** std::vector::pop_back() is documented as erasing the element. >> Squid::Vector does not. >> - under correct usage of std::vector the loop and delete X.back() would >> seem to be irrelevant. >> >> ** the pattern with loop seems to be equivalent to specs.clear() but far >> less efficient. Efficiency is saved here only by Squid::Vector not >> reallocating the items array in pop_back(). > > An explicit delete is necessary when storing raw pointers to objects. > Neither std::vector nor Squid's Vector [can] delete objects that > elements removed by clear() or pop_back() point to. > A generic container does not know whether it stores "raw pointers to > objects" or "just objects". When removing elements, it has to use the > element's default constructor or destructor. When an element is a raw > pointer, the default constructor and destructor do not do anything > [useful], so we have to delete those objects by hand using "delete" (or > use smart pointers instead).
The latter would probably be best, but it's out of scope for this effort IMO. > One could avoid the explicit loop using std::for_each. However, it would > not make things more efficient (or more clear) as far as vector memory > management is concerned. Maybe it would change something, but it'd be lost in the noise and is definitely not worth it. Thanks for the clarification, I had misread the reference. -- Francesco