Cleanup: use std::queue for UFS file I/O queues

diff --git a/src/fs/ufs/UFSStoreState.cc b/src/fs/ufs/UFSStoreState.cc
index 799af4e6a4daa319e3647752ace9885d4826eb18..22ebd4f3da00785555f80fb79696b49e34c5c46a 100644 (file)
--- a/src/fs/ufs/UFSStoreState.cc
+++ b/src/fs/ufs/UFSStoreState.cc
@@ -132,8 +132,9 @@ Fs::Ufs::UFSStoreState::read_(char *buf, size_t size, off_t aOffset, STRCB * aCa
     assert (aCallback);
 
     if (!theFile->canRead()) {
-        debugs(79, 3, "UFSStoreState::read_: queueing read because theFile can't read");
-        queueRead (buf, size, aOffset, aCallback, aCallbackData);
+        debugs(79, 3, "queueing read because theFile can't read");
+        assert(opening);
+        pending_reads.emplace(buf, size, aOffset, aCallback, aCallbackData);
         return;
     }
 
@@ -177,7 +178,8 @@ Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE
         return false;
     }
 
-    queueWrite(buf, size, aOffset, free_func);
+    debugs(79, 3, (void*)this << " queueing write of size " << size);
+    pending_writes.emplace(buf, size, aOffset, free_func);
     drainWriteQueue();
     return true;
 }
@@ -191,28 +193,20 @@ Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE
 void
 Fs::Ufs::UFSStoreState::doWrite()
 {
-    debugs(79, 3, HERE << this << " UFSStoreState::doWrite");
+    debugs(79, 3, (void*)this);
 
     assert(theFile->canWrite());
 
-    _queued_write *q = (_queued_write *)linklistShift(&pending_writes);
-
-    if (q == NULL) {
-        debugs(79, 3, HERE << this << " UFSStoreState::doWrite queue is empty");
+    if (pending_writes.empty()) {
+        debugs(79, 3, (void*)this << " write queue is empty");
         return;
     }
 
+    auto &q = pending_writes.front();
+
     if (theFile->error()) {
-        debugs(79, DBG_IMPORTANT,HERE << "avoid write on theFile with error");
-        debugs(79,3,HERE << "calling free_func for " << (void*) q->buf);
-        /*
-         * DPW 2006-05-24
-         * Note "free_func" is memNodeWriteComplete(), which doesn't
-         * really free the memory.  Instead it clears the node's
-         * write_pending flag.
-         */
-        q->free_func((void*)q->buf);
-        delete q;
+        debugs(79, DBG_IMPORTANT, MYNAME << " avoid write on theFile with error");
+        pending_writes.pop();
         return;
     }
 
@@ -226,10 +220,11 @@ Fs::Ufs::UFSStoreState::doWrite()
      * coming in.  For now let's just not use the writing flag at
      * all.
      */
-    debugs(79, 3, HERE << this << " calling theFile->write(" << q->size << ")");
+    debugs(79, 3, (void*)this << " calling theFile->write(" << q.size << ")");
 
-    theFile->write(new WriteRequest(q->buf, q->offset, q->size, q->free_func));
-    delete q;
+    theFile->write(new WriteRequest(q.buf, q.offset, q.size, q.free_func));
+    q.buf = nullptr; // prevent buf deletion on pop, its used by the above object
+    pending_writes.pop();
 }
 
 void
@@ -338,8 +333,6 @@ Fs::Ufs::UFSStoreState::UFSStoreState(SwapDir * SD, StoreEntry * anEntry, STIOCB
     closing(false),
     reading(false),
     writing(false),
-    pending_reads(NULL),
-    pending_writes(NULL),
     read_buf(NULL)
 {
     // StoreIOState inherited members
@@ -354,71 +347,44 @@ Fs::Ufs::UFSStoreState::UFSStoreState(SwapDir * SD, StoreEntry * anEntry, STIOCB
 
 Fs::Ufs::UFSStoreState::~UFSStoreState()
 {
-    assert(pending_reads == NULL);
-    assert(pending_writes == NULL);
+    assert(pending_reads.empty());
+    assert(pending_writes.empty());
 }
 
 void
 Fs::Ufs::UFSStoreState::freePending()
 {
-    _queued_read *qr;
-
-    while ((qr = (_queued_read *)linklistShift(&pending_reads))) {
-        cbdataReferenceDone(qr->callback_data);
-        delete qr;
-    }
-
-    debugs(79,3,HERE << "UFSStoreState::freePending: freed pending reads");
+    while (!pending_reads.empty())
+        pending_reads.pop();
+    debugs(79, 3, "freed pending reads");
 
-    _queued_write *qw;
-
-    while ((qw = (_queued_write *)linklistShift(&pending_writes))) {
-        if (qw->free_func)
-            qw->free_func(const_cast<char *>(qw->buf));
-        delete qw;
-    }
-
-    debugs(79,3,HERE << "UFSStoreState::freePending: freed pending writes");
+    while (!pending_writes.empty())
+        pending_writes.pop();
+    debugs(79, 3, "freed pending writes");
 }
 
 bool
 Fs::Ufs::UFSStoreState::kickReadQueue()
 {
-    _queued_read *q = (_queued_read *)linklistShift(&pending_reads);
-
-    if (NULL == q)
+    if (pending_reads.empty())
         return false;
 
-    debugs(79, 3, "UFSStoreState::kickReadQueue: reading queued request of " << q->size << " bytes");
+    auto &q = pending_reads.front();
 
-    void *cbdata;
+    debugs(79, 3, "reading queued request of " << q.size << " bytes");
 
-    if (cbdataReferenceValidDone(q->callback_data, &cbdata)) {
-        read_(q->buf, q->size, q->offset, q->callback, cbdata);
+    bool result = true;
+    void *cbdata;
+    if (cbdataReferenceValidDone(q.callback_data, &cbdata)) {
+        read_(q.buf, q.size, q.offset, q.callback, cbdata);
     } else {
-        debugs(79, 2, "UFSStoreState::kickReadQueue: this: " << this << " cbdataReferenceValidDone returned false." << " closing: " << closing << " flags.try_closing: " << flags.try_closing);
-        delete q;
-        return false;
+        debugs(79, 2, "this=" << (void*)this << " cbdataReferenceValidDone returned false." <<
+               " closing: " << closing << " flags.try_closing: " << flags.try_closing);
+        result = false;
     }
 
-    delete q;
-
-    return true;
-}
-
-void
-Fs::Ufs::UFSStoreState::queueRead(char *buf, size_t size, off_t aOffset, STRCB *callback_, void *callback_data_)
-{
-    debugs(79, 3, "UFSStoreState::queueRead: queueing read");
-    assert(opening);
-    assert (pending_reads == NULL);
-    _queued_read *q = new _queued_read;
-    q->buf = buf;
-    q->size = size;
-    q->offset = aOffset;
-    q->callback = callback_;
-    q->callback_data = cbdataReference(callback_data_);
-    linklistPush(&pending_reads, q);
+    pending_reads.pop(); // erase the front object
+    return result;
 }
 
 /*
@@ -445,9 +411,8 @@ Fs::Ufs::UFSStoreState::drainWriteQueue()
 
     flags.write_draining = true;
 
-    while (pending_writes != NULL) {
+    while (!pending_writes.empty())
         doWrite();
-    }
 
     flags.write_draining = false;
 
@@ -482,17 +447,3 @@ Fs::Ufs::UFSStoreState::tryClosing()
     theFile->close();
 }
 
-void
-Fs::Ufs::UFSStoreState::queueWrite(char const *buf, size_t size, off_t aOffset, FREE * free_func)
-{
-    debugs(79, 3, HERE << this << " UFSStoreState::queueWrite: queueing write of size " << size);
-
-    _queued_write *q;
-    q = new _queued_write;
-    q->buf = buf;
-    q->size = size;
-    q->offset = aOffset;
-    q->free_func = free_func;
-    linklistPush(&pending_writes, q);
-}
-

diff --git a/src/fs/ufs/UFSStoreState.h b/src/fs/ufs/UFSStoreState.h
index c722559335fa631037ba0290024accafd4c0b943..55ab7181daa61fc1b6b3021f011a5174b1506050 100644 (file)
--- a/src/fs/ufs/UFSStoreState.h
+++ b/src/fs/ufs/UFSStoreState.h
@@ -13,6 +13,8 @@
 #include "SquidList.h"
 #include "StoreIOState.h"
 
+#include <queue>
+
 namespace Fs
 {
 namespace Ufs
@@ -48,13 +50,16 @@ protected:
     {
         MEMPROXY_CLASS(UFSStoreState::_queued_read);
     public:
-        _queued_read() :
-            buf(nullptr),
-            size(0),
-            offset(0),
-            callback(nullptr),
-            callback_data(nullptr)
+        _queued_read(char *b, size_t s, off_t o, STRCB *cb, void *data) :
+            buf(b),
+            size(s),
+            offset(o),
+            callback(cb),
+            callback_data(cbdataReference(data))
         {}
+        ~_queued_read() {
+            cbdataReferenceDone(callback_data);
+        }
 
         char *buf;
         size_t size;
@@ -62,23 +67,35 @@ protected:
         STRCB *callback;
         void *callback_data;
     };
+    std::queue<Ufs::UFSStoreState::_queued_read> pending_reads;
 
     class _queued_write
     {
         MEMPROXY_CLASS(UFSStoreState::_queued_write);
     public:
-        _queued_write() :
-            buf(nullptr),
-            size(0),
-            offset(0),
-            free_func(nullptr)
+        _queued_write(const char *b, size_t s, off_t o, FREE *f) :
+            buf(b),
+            size(s),
+            offset(o),
+            free_func(f)
         {}
+        ~_queued_write() {
+           /*
+             * DPW 2006-05-24
+             * Note "free_func" is memNodeWriteComplete(), which doesn't
+             * really free the memory.  Instead it clears the node's
+             * write_pending flag.
+             */
+            if (free_func && buf)
+                free_func(const_cast<char *>(buf));
+        }
 
         char const *buf;
         size_t size;
         off_t offset;
         FREE *free_func;
     };
+    std::queue<Ufs::UFSStoreState::_queued_write> pending_writes;
 
     /** \todo These should be in the IO strategy */
 
@@ -98,10 +115,7 @@ protected:
          */
         bool try_closing;
     } flags;
-    link_list *pending_reads;
-    link_list *pending_writes;
-    void queueRead(char *, size_t, off_t, STRCB *, void *);
-    void queueWrite(char const *, size_t, off_t, FREE *);
+
     bool kickReadQueue();
     void drainWriteQueue();
     void tryClosing();