On 18/11/2015 7:31 p.m., Alex Rousskov wrote: > Hello, > > This first step towards bug #7 fix focuses on fixing several Store > API problems and polishing Store code. Many Store problems remain to be > solved, but this is a significant step forward IMO, even though no > functionality changes were intended during this step. The next steps > will address bug #7 itself. >
Thank you. Audit below. Since I'm still not completely familiar with Store I've not tried to do any deep analysis. It does appear to be largely cosmetic though. * in src/fs/ufs/UFSSwapDir.h and src/tests/TestSwapDir.h (at least) you have one class with a mix of override/non-override virtuals. - Kinkie and I have found the hard way earlier that some compilers error if only some virtual methods in a class heirarchy are marked with it. It is an all-or-nothing per-heirarchy situation with that keyword. - please coose whether to use it or not, and update appropriately before this goes in. * I seem to recall a fair amount of trouble over the local vs non-local vs unused/dead objects situation on a per-object basis rather than per-store. That was behind wantsLocalMemory parameter to Store::Controller::dereferenceIdle() which is being removed from some of the sub-calls. - is that a fixed problem now? * in src/store/Disks.cc please take advantage of the move to doxygen commen (\**) the functions that have prefix documentation already. * in src/tests/stub_store_search.cc please use nullptr - also since the stub is for StoreSearch.cc it should be named stub_StoreSearch.cc +1 provided the override issue is fixed. Though I don't think that will need another audit. Amos _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
