From 5d84beb598998a38d42075f33a1d0b3ae64f759e Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Tue, 27 Oct 2020 19:44:57 +0000 Subject: [PATCH] Avoid null pointer dereferences when dynamic_cast'ing to SwapDir (#743) Detected by Coverity. CID 1461158: Null pointer dereferences (FORWARD_NULL). When fixing these problems, we moved a few cache_dir iterations into Disks.cc by applying the "Only Store::Disks can iterate cache_dirs" design principle to the changed code. The Disks class is responsible for maintaining (and will eventually encapsulate all the knowledge of) the set of cache_dirs. Adjusted cache_cf.cc no longer depends on Disk.h. --- src/Makefile.am | 2 +- src/StoreFileSystem.cc | 10 ++ src/StoreFileSystem.h | 1 + src/cache_cf.cc | 104 +--------------- src/store.cc | 2 +- src/store/Controller.cc | 4 +- src/store/Controller.h | 4 +- src/store/Disks.cc | 203 +++++++++++++++++++++++-------- src/store/Disks.h | 14 ++- src/store_io.cc | 9 +- src/store_rebuild.cc | 9 +- src/tests/stub_libstore.cc | 12 +- src/tests/testRock.cc | 2 +- src/tests/testStoreController.cc | 2 +- src/tests/testStoreHashIndex.cc | 2 +- src/tests/testUfs.cc | 2 +- 16 files changed, 192 insertions(+), 190 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 59badedb3a..1a63903df5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1503,6 +1503,7 @@ tests_testStore_SOURCES = \ StatCounters.h \ StatHist.cc \ StatHist.h \ + StoreFileSystem.cc \ tests/testStore.cc \ tests/testStore.h \ tests/testStoreController.cc \ @@ -1781,7 +1782,6 @@ tests_testDiskIO_LDADD = \ dns/libdns.la \ base/libbase.la \ mem/libmem.la \ - store/libstore.la \ sbuf/libsbuf.la \ $(top_builddir)/lib/libmisccontainers.la \ $(top_builddir)/lib/libmiscencoding.la \ diff --git a/src/StoreFileSystem.cc b/src/StoreFileSystem.cc index cdb6855c2e..1c0772f66f 100644 --- a/src/StoreFileSystem.cc +++ b/src/StoreFileSystem.cc @@ -70,6 +70,16 @@ StoreFileSystem::FreeAllFs() } } +StoreFileSystem * +StoreFileSystem::FindByType(const char *type) +{ + for (const auto fs: FileSystems()) { + if (strcasecmp(type, fs->type()) == 0) + return fs; + } + return nullptr; +} + /* no filesystem is required to export statistics */ void StoreFileSystem::registerWithCacheManager(void) diff --git a/src/StoreFileSystem.h b/src/StoreFileSystem.h index 156e66abfc..79a6afeb5d 100644 --- a/src/StoreFileSystem.h +++ b/src/StoreFileSystem.h @@ -93,6 +93,7 @@ public: static void SetupAllFs(); static void FsAdd(StoreFileSystem &); static void FreeAllFs(); + static StoreFileSystem *FindByType(const char *type); static std::vector const &FileSystems(); typedef std::vector::iterator iterator; typedef std::vector::const_iterator const_iterator; diff --git a/src/cache_cf.cc b/src/cache_cf.cc index fc43e92942..4e8683627c 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -62,9 +62,7 @@ #include "SquidString.h" #include "ssl/ProxyCerts.h" #include "Store.h" -#include "store/Disk.h" #include "store/Disks.h" -#include "StoreFileSystem.h" #include "tools.h" #include "util.h" #include "wordlist.h" @@ -643,19 +641,6 @@ configDoConfigure(void) memConfigure(); /* Sanity checks */ - Config.cacheSwap.n_strands = 0; // no diskers by default - if (Config.cacheSwap.swapDirs == NULL) { - /* Memory-only cache probably in effect. */ - /* turn off the cache rebuild delays... */ - StoreController::store_dirs_rebuilding = 0; - } else if (InDaemonMode()) { // no diskers in non-daemon mode - for (int i = 0; i < Config.cacheSwap.n_configured; ++i) { - const RefCount sd = Config.cacheSwap.swapDirs[i]; - if (sd->needsDiskStrand()) - sd->disker = Config.workers + (++Config.cacheSwap.n_strands); - } - } - if (Debug::rotateNumber < 0) { Debug::rotateNumber = Config.Log.rotateNumber; } @@ -1853,17 +1838,7 @@ parse_http_header_replace(HeaderManglers **pm) static void dump_cachedir(StoreEntry * entry, const char *name, const Store::DiskConfig &swap) { - SwapDir *s; - int i; - assert (entry); - - for (i = 0; i < swap.n_configured; ++i) { - s = dynamic_cast(swap.swapDirs[i].getRaw()); - if (!s) continue; - storeAppendPrintf(entry, "%s %s %s", name, s->type(), s->path); - s->dump(*entry); - storeAppendPrintf(entry, "\n"); - } + Store::Disks::Dump(swap, *entry, name); } static int @@ -1981,84 +1956,11 @@ ParseAclWithAction(acl_access **access, const Acl::Answer &action, const char *d (*access)->add(rule, action); } -/* TODO: just return the object, the # is irrelevant */ -static int -find_fstype(char *type) -{ - for (size_t i = 0; i < StoreFileSystem::FileSystems().size(); ++i) - if (strcasecmp(type, StoreFileSystem::FileSystems().at(i)->type()) == 0) - return (int)i; - - return (-1); -} - static void parse_cachedir(Store::DiskConfig *swap) { - char *type_str = ConfigParser::NextToken(); - if (!type_str) { - self_destruct(); - return; - } - - char *path_str = ConfigParser::NextToken(); - if (!path_str) { - self_destruct(); - return; - } - - int fs = find_fstype(type_str); - if (fs < 0) { - debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: This proxy does not support the '" << type_str << "' cache type. Ignoring."); - return; - } - - /* reconfigure existing dir */ - - RefCount sd; - for (int i = 0; i < swap->n_configured; ++i) { - assert (swap->swapDirs[i].getRaw()); - - if ((strcasecmp(path_str, dynamic_cast(swap->swapDirs[i].getRaw())->path)) == 0) { - /* this is specific to on-fs Stores. The right - * way to handle this is probably to have a mapping - * from paths to stores, and have on-fs stores - * register with that, and lookip in that in their - * own setup logic. RBC 20041225. TODO. - */ - - sd = dynamic_cast(swap->swapDirs[i].getRaw()); - - if (strcmp(sd->type(), StoreFileSystem::FileSystems().at(fs)->type()) != 0) { - debugs(3, DBG_CRITICAL, "ERROR: Can't change type of existing cache_dir " << - sd->type() << " " << sd->path << " to " << type_str << ". Restart required"); - return; - } - - sd->reconfigure(); - return; - } - } - - /* new cache_dir */ - if (swap->n_configured > 63) { - /* 7 bits, signed */ - debugs(3, DBG_CRITICAL, "WARNING: There is a fixed maximum of 63 cache_dir entries Squid can handle."); - debugs(3, DBG_CRITICAL, "WARNING: '" << path_str << "' is one to many."); - self_destruct(); - return; - } - - allocate_new_swapdir(swap); - - swap->swapDirs[swap->n_configured] = StoreFileSystem::FileSystems().at(fs)->createSwapDir(); - - sd = dynamic_cast(swap->swapDirs[swap->n_configured].getRaw()); - - /* parse the FS parameters and options */ - sd->parse(swap->n_configured, path_str); - - ++swap->n_configured; + assert(swap); + Store::Disks::Parse(*swap); } static const char * diff --git a/src/store.cc b/src/store.cc index 450ae630cd..2a72a267e5 100644 --- a/src/store.cc +++ b/src/store.cc @@ -1303,7 +1303,7 @@ storeInit(void) void storeConfigure(void) { - Store::Root().updateLimits(); + Store::Root().configure(); } bool diff --git a/src/store/Controller.cc b/src/store/Controller.cc index 78809929a9..f83bdec9e0 100644 --- a/src/store/Controller.cc +++ b/src/store/Controller.cc @@ -195,9 +195,9 @@ Store::Controller::maxObjectSize() const } void -Store::Controller::updateLimits() +Store::Controller::configure() { - swapDir->updateLimits(); + swapDir->configure(); store_swap_high = (long) (((float) maxSize() * (float) Config.Swap.highWaterMark) / (float) 100); diff --git a/src/store/Controller.h b/src/store/Controller.h index 58094a9bc4..4b06dcc109 100644 --- a/src/store/Controller.h +++ b/src/store/Controller.h @@ -74,8 +74,8 @@ public: /// reduce the risk of selecting the wrong disk cache for the growing entry. int64_t accumulateMore(StoreEntry &) const; - /// slowly calculate (and cache) hi/lo watermarks and similar limits - void updateLimits(); + /// update configuration, including limits (re)calculation + void configure(); /// called when the entry is no longer needed by any transaction void handleIdleEntry(StoreEntry &); diff --git a/src/store/Disks.cc b/src/store/Disks.cc index e2ab46678c..5aebd0ad91 100644 --- a/src/store/Disks.cc +++ b/src/store/Disks.cc @@ -9,24 +9,31 @@ /* DEBUG: section 47 Store Directory Routines */ #include "squid.h" +#include "cache_cf.h" +#include "ConfigParser.h" #include "Debug.h" #include "globals.h" #include "profiler/Profiler.h" +#include "sbuf/Stream.h" #include "SquidConfig.h" #include "Store.h" #include "store/Disk.h" #include "store/Disks.h" +#include "StoreFileSystem.h" #include "store_rebuild.h" #include "swap_log_op.h" +#include "tools.h" #include "util.h" // for tvSubDsec() which should be in SquidTime.h +typedef SwapDir *STDIRSELECT(const StoreEntry *e); + static STDIRSELECT storeDirSelectSwapDirRoundRobin; static STDIRSELECT storeDirSelectSwapDirLeastLoad; /** * This function pointer is set according to 'store_dir_select_algorithm' * in squid.conf. */ -STDIRSELECT *storeDirSelectSwapDir = storeDirSelectSwapDirLeastLoad; +static STDIRSELECT *storeDirSelectSwapDir = storeDirSelectSwapDirLeastLoad; /// The entry size to use for Disk::canStore() size limit checks. /// This is an optimization to avoid similar calculations in every cache_dir. @@ -46,12 +53,23 @@ objectSizeForDirSelection(const StoreEntry &entry) return minSize; } +/// TODO: Remove when cache_dir-iterating functions are converted to Disks methods +static SwapDir & +SwapDirByIndex(const int i) +{ + assert(i >= 0); + assert(i < Config.cacheSwap.n_allocated); + const auto sd = INDEXSD(i); + assert(sd); + return *sd; +} + /** * This new selection scheme simply does round-robin on all SwapDirs. * A SwapDir is skipped if it is over the max_size (100%) limit, or * overloaded. */ -static int +static SwapDir * storeDirSelectSwapDirRoundRobin(const StoreEntry * e) { const int64_t objsize = objectSizeForDirSelection(*e); @@ -64,20 +82,20 @@ storeDirSelectSwapDirRoundRobin(const StoreEntry * e) for (int i = 0; i < Config.cacheSwap.n_configured; ++i) { const int dirn = (firstCandidate + i) % Config.cacheSwap.n_configured; - const SwapDir *sd = dynamic_cast(INDEXSD(dirn)); + auto &dir = SwapDirByIndex(dirn); int load = 0; - if (!sd->canStore(*e, objsize, load)) + if (!dir.canStore(*e, objsize, load)) continue; if (load < 0 || load > 1000) { continue; } - return dirn; + return &dir; } - return -1; + return nullptr; } /** @@ -93,24 +111,23 @@ storeDirSelectSwapDirRoundRobin(const StoreEntry * e) * ALL swapdirs, regardless of state. Again, this is a hack while * we sort out the real usefulness of this algorithm. */ -static int +static SwapDir * storeDirSelectSwapDirLeastLoad(const StoreEntry * e) { int64_t most_free = 0; int64_t best_objsize = -1; int least_load = INT_MAX; int load; - int dirn = -1; + SwapDir *selectedDir = nullptr; int i; - RefCount SD; const int64_t objsize = objectSizeForDirSelection(*e); for (i = 0; i < Config.cacheSwap.n_configured; ++i) { - SD = dynamic_cast(INDEXSD(i)); - SD->flags.selected = false; + auto &sd = SwapDirByIndex(i); + sd.flags.selected = false; - if (!SD->canStore(*e, objsize, load)) + if (!sd.canStore(*e, objsize, load)) continue; if (load < 0 || load > 1000) @@ -119,7 +136,7 @@ storeDirSelectSwapDirLeastLoad(const StoreEntry * e) if (load > least_load) continue; - const int64_t cur_free = SD->maxSize() - SD->currentSize(); + const int64_t cur_free = sd.maxSize() - sd.currentSize(); /* If the load is equal, then look in more details */ if (load == least_load) { @@ -127,8 +144,8 @@ storeDirSelectSwapDirLeastLoad(const StoreEntry * e) if (best_objsize != -1) { // cache_dir with the smallest max-size gets the known-size object // cache_dir with the largest max-size gets the unknown-size object - if ((objsize != -1 && SD->maxObjectSize() > best_objsize) || - (objsize == -1 && SD->maxObjectSize() < best_objsize)) + if ((objsize != -1 && sd.maxObjectSize() > best_objsize) || + (objsize == -1 && sd.maxObjectSize() < best_objsize)) continue; } @@ -138,15 +155,15 @@ storeDirSelectSwapDirLeastLoad(const StoreEntry * e) } least_load = load; - best_objsize = SD->maxObjectSize(); + best_objsize = sd.maxObjectSize(); most_free = cur_free; - dirn = i; + selectedDir = &sd; } - if (dirn >= 0) - dynamic_cast(INDEXSD(dirn))->flags.selected = true; + if (selectedDir) + selectedDir->flags.selected = true; - return dirn; + return selectedDir; } Store::Disks::Disks(): @@ -159,15 +176,13 @@ Store::Disks::Disks(): SwapDir * Store::Disks::store(int const x) const { - return INDEXSD(x); + return &SwapDirByIndex(x); } SwapDir & Store::Disks::Dir(const int i) { - SwapDir *sd = INDEXSD(i); - assert(sd); - return *sd; + return SwapDirByIndex(i); } int @@ -225,11 +240,11 @@ Store::Disks::get(const cache_key *key) static int idx = 0; for (int n = 0; n < cacheDirs; ++n) { idx = (idx + 1) % cacheDirs; - SwapDir *sd = dynamic_cast(INDEXSD(idx)); - if (!sd->active()) + auto &sd = Dir(idx); + if (!sd.active()) continue; - if (StoreEntry *e = sd->get(key)) { + if (auto e = sd.get(key)) { debugs(20, 7, "cache_dir " << idx << " has: " << *e); return e; } @@ -363,14 +378,25 @@ Store::Disks::maxObjectSize() const } void -Store::Disks::updateLimits() +Store::Disks::configure() { + if (!Config.cacheSwap.swapDirs) + Controller::store_dirs_rebuilding = 0; // nothing to index + largestMinimumObjectSize = -1; largestMaximumObjectSize = -1; secondLargestMaximumObjectSize = -1; + Config.cacheSwap.n_strands = 0; + for (int i = 0; i < Config.cacheSwap.n_configured; ++i) { - const auto &disk = Dir(i); + auto &disk = Dir(i); + if (disk.needsDiskStrand()) { + assert(InDaemonMode()); + // XXX: Do not pretend to support disk.disker changes during reconfiguration + disk.disker = Config.workers + (++Config.cacheSwap.n_strands); + } + if (!disk.active()) continue; @@ -386,6 +412,70 @@ Store::Disks::updateLimits() } } +void +Store::Disks::Parse(DiskConfig &swap) +{ + const auto typeStr = ConfigParser::NextToken(); + if (!typeStr) + throw TextException("missing cache_dir parameter: storage type", Here()); + + const auto pathStr = ConfigParser::NextToken(); + if (!pathStr) + throw TextException("missing cache_dir parameter: directory name", Here()); + + const auto fs = StoreFileSystem::FindByType(typeStr); + if (!fs) { + debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: This proxy does not support the '" << typeStr << "' cache type. Ignoring."); + return; + } + + const auto fsType = fs->type(); + + // check for the existing cache_dir + // XXX: This code mistreats duplicated cache_dir entries (that should be fatal). + for (int i = 0; i < swap.n_configured; ++i) { + auto &disk = Dir(i); + if ((strcasecmp(pathStr, disk.path)) == 0) { + /* this is specific to on-fs Stores. The right + * way to handle this is probably to have a mapping + * from paths to stores, and have on-fs stores + * register with that, and lookip in that in their + * own setup logic. RBC 20041225. TODO. + */ + + if (strcmp(disk.type(), fsType) == 0) + disk.reconfigure(); + else + debugs(3, DBG_CRITICAL, "ERROR: Can't change type of existing cache_dir " << + disk.type() << " " << disk.path << " to " << fsType << ". Restart required"); + + return; + } + } + + const int cacheDirCountLimit = 64; // StoreEntry::swap_dirn is a signed 7-bit integer + if (swap.n_configured >= cacheDirCountLimit) + throw TextException(ToSBuf("Squid cannot handle more than ", cacheDirCountLimit, " cache_dir directives"), Here()); + + // create a new cache_dir + allocate_new_swapdir(swap); + swap.swapDirs[swap.n_configured] = fs->createSwapDir(); + auto &disk = Dir(swap.n_configured); + disk.parse(swap.n_configured, pathStr); + ++swap.n_configured; +} + +void +Store::Disks::Dump(const DiskConfig &swap, StoreEntry &entry, const char *name) +{ + for (int i = 0; i < swap.n_configured; ++i) { + const auto &disk = Dir(i); + storeAppendPrintf(&entry, "%s %s %s", name, disk.type(), disk.path); + disk.dump(entry); + storeAppendPrintf(&entry, "\n"); + } +} + int64_t Store::Disks::accumulateMore(const StoreEntry &entry) const { @@ -549,7 +639,7 @@ bool Store::Disks::updateAnchored(StoreEntry &entry) { return entry.hasDisk() && - Dir(entry.swap_dirn).updateAnchored(entry); + entry.disk().updateAnchored(entry); } bool @@ -565,6 +655,12 @@ Store::Disks::SmpAware() return false; } +SwapDir * +Store::Disks::SelectSwapDir(const StoreEntry *e) +{ + return storeDirSelectSwapDir(e); +} + bool Store::Disks::hasReadableEntry(const StoreEntry &e) const { @@ -578,14 +674,14 @@ void storeDirOpenSwapLogs() { for (int dirn = 0; dirn < Config.cacheSwap.n_configured; ++dirn) - INDEXSD(dirn)->openLog(); + SwapDirByIndex(dirn).openLog(); } void storeDirCloseSwapLogs() { for (int dirn = 0; dirn < Config.cacheSwap.n_configured; ++dirn) - INDEXSD(dirn)->closeLog(); + SwapDirByIndex(dirn).closeLog(); } /** @@ -605,7 +701,6 @@ storeDirWriteCleanLogs(int reopen) struct timeval start; double dt; - RefCount sd; int dirn; int notdone = 1; @@ -623,10 +718,10 @@ storeDirWriteCleanLogs(int reopen) start = current_time; for (dirn = 0; dirn < Config.cacheSwap.n_configured; ++dirn) { - sd = dynamic_cast(INDEXSD(dirn)); + auto &sd = SwapDirByIndex(dirn); - if (sd->writeCleanStart() < 0) { - debugs(20, DBG_IMPORTANT, "log.clean.start() failed for dir #" << sd->index); + if (sd.writeCleanStart() < 0) { + debugs(20, DBG_IMPORTANT, "log.clean.start() failed for dir #" << sd.index); continue; } } @@ -640,22 +735,22 @@ storeDirWriteCleanLogs(int reopen) notdone = 0; for (dirn = 0; dirn < Config.cacheSwap.n_configured; ++dirn) { - sd = dynamic_cast(INDEXSD(dirn)); + auto &sd = SwapDirByIndex(dirn); - if (NULL == sd->cleanLog) + if (!sd.cleanLog) continue; - e = sd->cleanLog->nextEntry(); + e = sd.cleanLog->nextEntry(); if (!e) continue; notdone = 1; - if (!sd->canLog(*e)) + if (!sd.canLog(*e)) continue; - sd->cleanLog->write(*e); + sd.cleanLog->write(*e); if ((++n & 0xFFFF) == 0) { getCurrentTime(); @@ -667,7 +762,7 @@ storeDirWriteCleanLogs(int reopen) /* Flush */ for (dirn = 0; dirn < Config.cacheSwap.n_configured; ++dirn) - dynamic_cast(INDEXSD(dirn))->writeCleanDone(); + SwapDirByIndex(dirn).writeCleanDone(); if (reopen) storeDirOpenSwapLogs(); @@ -686,21 +781,21 @@ storeDirWriteCleanLogs(int reopen) /* Globals that should be converted to static Store::Disks methods */ void -allocate_new_swapdir(Store::DiskConfig *swap) +allocate_new_swapdir(Store::DiskConfig &swap) { - if (!swap->swapDirs) { - swap->n_allocated = 4; - swap->swapDirs = new SwapDir::Pointer[swap->n_allocated]; + if (!swap.swapDirs) { + swap.n_allocated = 4; + swap.swapDirs = new SwapDir::Pointer[swap.n_allocated]; } - if (swap->n_allocated == swap->n_configured) { - swap->n_allocated <<= 1; - const auto tmp = new SwapDir::Pointer[swap->n_allocated]; - for (int i = 0; i < swap->n_configured; ++i) { - tmp[i] = swap->swapDirs[i]; + if (swap.n_allocated == swap.n_configured) { + swap.n_allocated <<= 1; + const auto tmp = new SwapDir::Pointer[swap.n_allocated]; + for (int i = 0; i < swap.n_configured; ++i) { + tmp[i] = swap.swapDirs[i]; } - delete[] swap->swapDirs; - swap->swapDirs = tmp; + delete[] swap.swapDirs; + swap.swapDirs = tmp; } } @@ -758,6 +853,6 @@ storeDirSwapLog(const StoreEntry * e, int op) e->swap_dirn << " " << std::hex << std::uppercase << std::setfill('0') << std::setw(8) << e->swap_filen); - dynamic_cast(INDEXSD(e->swap_dirn))->logEntry(*e, op); + e->disk().logEntry(*e, op); } diff --git a/src/store/Disks.h b/src/store/Disks.h index 194d45b62c..76c2cdce54 100644 --- a/src/store/Disks.h +++ b/src/store/Disks.h @@ -42,14 +42,19 @@ public: virtual void evictIfFound(const cache_key *) override; virtual int callback() override; - /// slowly calculate (and cache) hi/lo watermarks and similar limits - void updateLimits(); + /// update configuration, including limits (re)calculation + void configure(); + /// parses a single cache_dir configuration line + static void Parse(DiskConfig &); + /// prints the configuration into the provided StoreEntry + static void Dump(const DiskConfig &, StoreEntry &, const char *name); /// Additional unknown-size entry bytes required by disks in order to /// reduce the risk of selecting the wrong disk cache for the growing entry. int64_t accumulateMore(const StoreEntry&) const; /// whether any disk cache is SMP-aware static bool SmpAware(); + static SwapDir *SelectSwapDir(const StoreEntry *); /// whether any of disk caches has entry with e.key bool hasReadableEntry(const StoreEntry &) const; @@ -71,12 +76,9 @@ int storeDirWriteCleanLogs(int reopen); void storeDirCloseSwapLogs(void); /* Globals that should be converted to static Store::Disks methods */ -void allocate_new_swapdir(Store::DiskConfig *swap); +void allocate_new_swapdir(Store::DiskConfig &swap); void free_cachedir(Store::DiskConfig *swap); -/* Globals that should be converted to Store::Disks private data members */ -typedef int STDIRSELECT(const StoreEntry *e); -extern STDIRSELECT *storeDirSelectSwapDir; /* Globals that should be moved to some Store::UFS-specific logging module */ void storeDirSwapLog(const StoreEntry *e, int op); diff --git a/src/store_io.cc b/src/store_io.cc index 7bcf3a1441..acb8f7e967 100644 --- a/src/store_io.cc +++ b/src/store_io.cc @@ -32,19 +32,16 @@ storeCreate(StoreEntry * e, StoreIOState::STFNCB * file_callback, StoreIOState:: * Pick the swapdir * We assume that the header has been packed by now .. */ - const sdirno dirn = storeDirSelectSwapDir(e); + const auto sd = Store::Disks::SelectSwapDir(e); - if (dirn == -1) { + if (!sd) { debugs(20, 2, "storeCreate: no swapdirs for " << *e); ++store_io_stats.create.select_fail; return NULL; } - debugs(20, 2, "storeCreate: Selected dir " << dirn << " for " << *e); - SwapDir *SD = dynamic_cast(INDEXSD(dirn)); - /* Now that we have a fs to use, call its storeCreate function */ - StoreIOState::Pointer sio = SD->createStoreIO(*e, file_callback, close_callback, callback_data); + StoreIOState::Pointer sio = sd->createStoreIO(*e, file_callback, close_callback, callback_data); if (sio == NULL) ++store_io_stats.create.create_fail; diff --git a/src/store_rebuild.cc b/src/store_rebuild.cc index 71f343386e..2f1e40ed5e 100644 --- a/src/store_rebuild.cc +++ b/src/store_rebuild.cc @@ -47,13 +47,6 @@ StoreRebuildData::updateStartTime(const timeval &dirStartTime) startTime = started() ? std::min(startTime, dirStartTime) : dirStartTime; } -static int -storeCleanupDoubleCheck(StoreEntry * e) -{ - SwapDir *SD = dynamic_cast(INDEXSD(e->swap_dirn)); - return (SD->doubleCheck(*e)); -} - static void storeCleanup(void *) { @@ -86,7 +79,7 @@ storeCleanup(void *) continue; if (opt_store_doublecheck) - if (storeCleanupDoubleCheck(e)) + if (e->disk().doubleCheck(*e)) ++store_errors; EBIT_SET(e->flags, ENTRY_VALIDATED); diff --git a/src/tests/stub_libstore.cc b/src/tests/stub_libstore.cc index bf27ae66fb..200b0b0f7e 100644 --- a/src/tests/stub_libstore.cc +++ b/src/tests/stub_libstore.cc @@ -37,7 +37,7 @@ bool Controller::markedForDeletion(const cache_key *) const STUB_RETVAL(false) bool Controller::markedForDeletionAndAbandoned(const StoreEntry &) const STUB_RETVAL(false) bool Controller::hasReadableDiskEntry(const StoreEntry &) const STUB_RETVAL(false) int64_t Controller::accumulateMore(StoreEntry &) const STUB_RETVAL(0) -void Controller::updateLimits() STUB +void Controller::configure() STUB void Controller::handleIdleEntry(StoreEntry &) STUB void Controller::freeMemorySpace(const int) STUB void Controller::memoryOut(StoreEntry &, const bool) STUB @@ -122,17 +122,19 @@ bool Disks::updateAnchored(StoreEntry &) STUB_RETVAL(false) void Disks::evictCached(StoreEntry &) STUB void Disks::evictIfFound(const cache_key *) STUB int Disks::callback() STUB_RETVAL(0) -void Disks::updateLimits() STUB +void Disks::configure() STUB int64_t Disks::accumulateMore(const StoreEntry&) const STUB_RETVAL(0) bool Disks::SmpAware() STUB_RETVAL(false) bool Disks::hasReadableEntry(const StoreEntry &) const STUB_RETVAL(false) +void Disks::Parse(DiskConfig &) STUB +void Disks::Dump(const DiskConfig &, StoreEntry &, const char *name) STUB +SwapDir *Disks::SelectSwapDir(const StoreEntry *) STUB_RETVAL(nullptr) } void storeDirOpenSwapLogs(void) STUB int storeDirWriteCleanLogs(int) STUB_RETVAL(0) void storeDirCloseSwapLogs(void) STUB -void allocate_new_swapdir(Store::DiskConfig *) STUB -void free_cachedir(Store::DiskConfig *) STUB -STDIRSELECT *storeDirSelectSwapDir = nullptr; +void allocate_new_swapdir(Store::DiskConfig &) STUB +void free_cachedir(Store::DiskConfig *) STUB; void storeDirSwapLog(const StoreEntry *, int) STUB #include "store/LocalSearch.h" diff --git a/src/tests/testRock.cc b/src/tests/testRock.cc index d9634d3186..f6a7df5e95 100644 --- a/src/tests/testRock.cc +++ b/src/tests/testRock.cc @@ -45,7 +45,7 @@ static char cwd[MAXPATHLEN]; static void addSwapDir(testRock::SwapDirPointer aStore) { - allocate_new_swapdir(&Config.cacheSwap); + allocate_new_swapdir(Config.cacheSwap); Config.cacheSwap.swapDirs[Config.cacheSwap.n_configured] = aStore.getRaw(); ++Config.cacheSwap.n_configured; } diff --git a/src/tests/testStoreController.cc b/src/tests/testStoreController.cc index a9b4faa90e..99a6502103 100644 --- a/src/tests/testStoreController.cc +++ b/src/tests/testStoreController.cc @@ -21,7 +21,7 @@ CPPUNIT_TEST_SUITE_REGISTRATION( testStoreController ); static void addSwapDir(TestSwapDirPointer aStore) { - allocate_new_swapdir(&Config.cacheSwap); + allocate_new_swapdir(Config.cacheSwap); Config.cacheSwap.swapDirs[Config.cacheSwap.n_configured] = aStore.getRaw(); ++Config.cacheSwap.n_configured; } diff --git a/src/tests/testStoreHashIndex.cc b/src/tests/testStoreHashIndex.cc index acc63a607b..4d6884c2b6 100644 --- a/src/tests/testStoreHashIndex.cc +++ b/src/tests/testStoreHashIndex.cc @@ -21,7 +21,7 @@ CPPUNIT_TEST_SUITE_REGISTRATION( testStoreHashIndex ); static void addSwapDir(TestSwapDirPointer aStore) { - allocate_new_swapdir(&Config.cacheSwap); + allocate_new_swapdir(Config.cacheSwap); Config.cacheSwap.swapDirs[Config.cacheSwap.n_configured] = aStore.getRaw(); ++Config.cacheSwap.n_configured; } diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc index d5bad5d9d2..9782ed282b 100644 --- a/src/tests/testUfs.cc +++ b/src/tests/testUfs.cc @@ -34,7 +34,7 @@ extern REMOVALPOLICYCREATE createRemovalPolicy_lru; /* XXX fails with --enable-r static void addSwapDir(MySwapDirPointer aStore) { - allocate_new_swapdir(&Config.cacheSwap); + allocate_new_swapdir(Config.cacheSwap); Config.cacheSwap.swapDirs[Config.cacheSwap.n_configured] = aStore.getRaw(); ++Config.cacheSwap.n_configured; } -- 2.47.2