From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 28 Apr 2014 20:30:43 +0000 (-0600)
Subject: Polished to address squid-dev review comments.
X-Git-Tag: SQUID_3_5_0_1~170^2~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7015a14919dd6a4a969fde1c7cd9e196e2de99e2;p=thirdparty%2Fsquid.git

Polished to address squid-dev review comments.
---

diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc
index 87902ee893..c724d982e3 100644
--- a/src/DiskIO/IpcIo/IpcIoFile.cc
+++ b/src/DiskIO/IpcIo/IpcIoFile.cc
@@ -18,6 +18,7 @@
 #include "ipc/Queue.h"
 #include "ipc/StrandSearch.h"
 #include "ipc/UdsOp.h"
+#include "SBuf.h"
 #include "SquidConfig.h"
 #include "SquidTime.h"
 #include "StatCounters.h"
@@ -43,8 +44,8 @@ std::auto_ptr<IpcIoFile::Queue> IpcIoFile::queue;
 
 bool IpcIoFile::DiskerHandleMoreRequestsScheduled = false;
 
-static bool DiskerOpen(const String &path, int flags, mode_t mode);
-static void DiskerClose(const String &path);
+static bool DiskerOpen(const SBuf &path, int flags, mode_t mode);
+static void DiskerClose(const SBuf &path);
 
 /// IpcIo wrapper for debugs() streams; XXX: find a better class name
 struct SipcIo {
@@ -98,7 +99,7 @@ IpcIoFile::open(int flags, mode_t mode, RefCount<IORequestor> callback)
         queue.reset(new Queue(ShmLabel, IamWorkerProcess() ? Queue::groupA : Queue::groupB, KidIdentifier));
 
     if (IamDiskProcess()) {
-        error_ = !DiskerOpen(dbName, flags, mode);
+        error_ = !DiskerOpen(SBuf(dbName.termedBuf()), flags, mode);
         if (error_)
             return;
 
@@ -177,7 +178,7 @@ IpcIoFile::close()
     assert(ioRequestor != NULL);
 
     if (IamDiskProcess())
-        DiskerClose(dbName);
+        DiskerClose(SBuf(dbName.termedBuf()));
     // XXX: else nothing to do?
 
     ioRequestor->closeCompleted();
@@ -636,7 +637,7 @@ IpcIoPendingRequest::completeIo(IpcIoMsg *const response)
 
 /* XXX: disker code that should probably be moved elsewhere */
 
-static String DbName; ///< full db file name
+static SBuf DbName; ///< full db file name
 static int TheFile = -1; ///< db file descriptor
 
 static void
@@ -884,12 +885,12 @@ IpcIoFile::DiskerHandleRequest(const int workerId, IpcIoMsg &ipcIo)
 }
 
 static bool
-DiskerOpen(const String &path, int flags, mode_t mode)
+DiskerOpen(const SBuf &path, int flags, mode_t mode)
 {
     assert(TheFile < 0);
 
     DbName = path;
-    TheFile = file_open(DbName.termedBuf(), flags);
+    TheFile = file_open(DbName.c_str(), flags);
 
     if (TheFile < 0) {
         const int xerrno = errno;
@@ -899,12 +900,12 @@ DiskerOpen(const String &path, int flags, mode_t mode)
     }
 
     ++store_open_disk_fd;
-    debugs(79,3, HERE << "rock db opened " << DbName << ": FD " << TheFile);
+    debugs(79,3, "rock db opened " << DbName << ": FD " << TheFile);
     return true;
 }
 
 static void
-DiskerClose(const String &path)
+DiskerClose(const SBuf &path)
 {
     if (TheFile >= 0) {
         file_close(TheFile);
@@ -912,7 +913,7 @@ DiskerClose(const String &path)
         TheFile = -1;
         --store_open_disk_fd;
     }
-    DbName.clean();
+    DbName.clear();
 }
 
 /// reports our needs for shared memory pages to Ipc::Mem::Pages
diff --git a/src/Store.h b/src/Store.h
index 6a4c191c2b..a48d7d584c 100644
--- a/src/Store.h
+++ b/src/Store.h
@@ -120,10 +120,10 @@ public:
     bool swappingOut() const { return swap_status == SWAPOUT_WRITING; }
     void swapOutFileClose(int how);
     const char *url() const;
-    /// generally cachable (checks agnostic to disk/RAM-specific requirements)
-    /// common part of mayStartSwapOut() and memoryCachable()
-    /// TODO: Make private so only those two methods can call this.
-    int checkCachable();
+    /// Satisfies cachability requirements shared among disk and RAM caches.
+    /// Encapsulates common checks of mayStartSwapOut() and memoryCachable().
+    /// TODO: Rename and make private so only those two methods can call this.
+    bool checkCachable();
     int checkNegativeHit() const;
     int locked() const;
     int validToSend() const;
diff --git a/src/store.cc b/src/store.cc
index 206e28d86f..62371cbdd3 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -950,7 +950,7 @@ StoreEntry::checkTooSmall()
 }
 
 // TODO: move "too many open..." checks outside -- we are called too early/late
-int
+bool
 StoreEntry::checkCachable()
 {
     // XXX: This method is used for both memory and disk caches, but some
diff --git a/src/tests/stub_store.cc b/src/tests/stub_store.cc
index fd2bb4819a..930653697e 100644
--- a/src/tests/stub_store.cc
+++ b/src/tests/stub_store.cc
@@ -46,7 +46,7 @@ void StoreEntry::purgeMem() STUB
 void StoreEntry::swapOut() STUB
 void StoreEntry::swapOutFileClose(int how) STUB
 const char *StoreEntry::url() const STUB_RETVAL(NULL)
-int StoreEntry::checkCachable() STUB_RETVAL(0)
+bool StoreEntry::checkCachable() STUB_RETVAL(0)
 int StoreEntry::checkNegativeHit() const STUB_RETVAL(0)
 int StoreEntry::locked() const STUB_RETVAL(0)
 int StoreEntry::validToSend() const STUB_RETVAL(0)