]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Avoid null pointer dereferences when dynamic_cast'ing to SwapDir (#743)
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Tue, 27 Oct 2020 19:44:57 +0000 (19:44 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Tue, 27 Oct 2020 20:09:51 +0000 (20:09 +0000)
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.

16 files changed:
src/Makefile.am
src/StoreFileSystem.cc
src/StoreFileSystem.h
src/cache_cf.cc
src/store.cc
src/store/Controller.cc
src/store/Controller.h
src/store/Disks.cc
src/store/Disks.h
src/store_io.cc
src/store_rebuild.cc
src/tests/stub_libstore.cc
src/tests/testRock.cc
src/tests/testStoreController.cc
src/tests/testStoreHashIndex.cc
src/tests/testUfs.cc

index 59badedb3a003fa4b14f9acc0c408eda7354b789..1a63903df573c64ad61e8136da2e5fa453281f7b 100644 (file)
@@ -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 \
index cdb6855c2e051cbd023029e278254ca5974066f4..1c0772f66f7bd4e0b8277f39bc5f531be0687117 100644 (file)
@@ -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)
index 156e66abfc4e2966656eddf9ca050622d5b4eb01..79a6afeb5d29ee2c6a5b01fb456fdd4c6532d984 100644 (file)
@@ -93,6 +93,7 @@ public:
     static void SetupAllFs();
     static void FsAdd(StoreFileSystem &);
     static void FreeAllFs();
+    static StoreFileSystem *FindByType(const char *type);
     static std::vector<StoreFileSystem*> const &FileSystems();
     typedef std::vector<StoreFileSystem*>::iterator iterator;
     typedef std::vector<StoreFileSystem*>::const_iterator const_iterator;
index fc43e92942f54f98aedb1363f33d149e001a1d59..4e8683627c9f5352dda0399fbdbeca7a0cf30e44 100644 (file)
@@ -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<SwapDir> 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<SwapDir *>(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<SwapDir> sd;
-    for (int i = 0; i < swap->n_configured; ++i) {
-        assert (swap->swapDirs[i].getRaw());
-
-        if ((strcasecmp(path_str, dynamic_cast<SwapDir *>(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<SwapDir *>(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<SwapDir *>(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 *
index 450ae630cd92fe0403f1cec8a02e2fd727bd37e0..2a72a267e585a75c012ffdf4256cf17f16a3089e 100644 (file)
@@ -1303,7 +1303,7 @@ storeInit(void)
 void
 storeConfigure(void)
 {
-    Store::Root().updateLimits();
+    Store::Root().configure();
 }
 
 bool
index 78809929a98833988cde0bf1b0d05432b37a46e9..f83bdec9e0265fa5d7a1334f37848f93f98297bb 100644 (file)
@@ -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);
index 58094a9bc4f62e339b9f6f2693ecd1c564a0407a..4b06dcc1096161d42a33b341aa57da33c289dfb2 100644 (file)
@@ -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 &);
index e2ab46678c398e38e47f83e432323535fbc63821..5aebd0ad91c9475bfdf684eac3457af6d9dd70c0 100644 (file)
@@ -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<SwapDir*>(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<SwapDir> SD;
 
     const int64_t objsize = objectSizeForDirSelection(*e);
 
     for (i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        SD = dynamic_cast<SwapDir *>(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<SwapDir *>(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<SwapDir*>(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<SwapDir> 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<SwapDir *>(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<SwapDir *>(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<SwapDir *>(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<SwapDir *>(INDEXSD(e->swap_dirn))->logEntry(*e, op);
+    e->disk().logEntry(*e, op);
 }
 
index 194d45b62ca4e89712e3073af1f794617fb340e2..76c2cdce5410b571c6b1193965f77524af21a631 100644 (file)
@@ -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);
index 7bcf3a1441ba4db1b47b0646656e6a32c86e4372..acb8f7e9676412226975005e06077e585d24e445 100644 (file)
@@ -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<SwapDir *>(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;
index 71f343386edc22c99525db35285c7733af273018..2f1e40ed5ec640f8750485ac3f5716ddb2d8a2eb 100644 (file)
@@ -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<SwapDir *>(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);
index bf27ae66fb2542ff1e62d62d09b4360514762aa0..200b0b0f7e83665259d06b78f05bb63872838b82 100644 (file)
@@ -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"
index d9634d31862fd6c50ddef882cb3f281c524e0a6e..f6a7df5e95e0878de8908894b9f4353bfe97f24d 100644 (file)
@@ -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;
 }
index a9b4faa90ed21798cc0aa0fabbdbd1c51eac1a17..99a6502103ab9d72c2d5f50e036ddc2776bb9636 100644 (file)
@@ -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;
 }
index acc63a607bf1f66a2515650f737508a36dc52125..4d6884c2b6309a45ed6529e1be38778de27916ea 100644 (file)
@@ -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;
 }
index d5bad5d9d2cf0085286bfad2a14c0bb40ef33f67..9782ed282bd240e7b2c0190bde38e6a534e31d42 100644 (file)
@@ -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;
 }