On 5/02/2014 8:57 a.m., Kinkie wrote: > On Sun, Feb 2, 2014 at 10:42 PM, Amos Jeffries <[email protected]> wrote: >> On 2014-02-03 08:06, Kinkie wrote: >>> >>> Hi, >>> the attached patch (merge from lp:~squid/squid/vector-refactor) is >>> an attempt to refactor Vector and its clients so that: >>> - clients of Vector don't break layering >>> - the Vector API more closely matches std::vector >>> >>> The eventual aim is to replace Vector with std::vector, only if some >>> more accurate measurements (in the lp:~squid/squid/vector-to-stdvector >>> branch) show that this wouldn't cause performance degradations. >>> >>> Don't be fooled by the size of the patch; it is big because there's >>> lots of context. >>> >>> Thanks >> >> >> 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(). > > Both left in after Alex' comments.
Okay. > >> in src/HttpHeader.cc: >> * please replace C-style casts with C++ casts in lines changed. static_cast >> would be appropriate in several places in this patch, if needed at all. > > Done. > >> in src/base/Vector.h >> * still need to remove operator +=. There are likely more conversions to >> push_back() hiding as a result. > > Done. There were two. > > v2 attached. > Okay. Looks good. +1. Amos
