[squid-dev] [PATCH] Remove SquidList / link_list

Amos Jeffries squid3 at treenet.co.nz
Thu Apr 14 12:31:28 UTC 2016


This patch replaces the remaining use of Squid custom link_list type
with STL std::queue or std::list templates. Removing the now unneeded
custom type completely.

It builds on the previous libmem old_api cleanup patch and has yet to be
run tested, though the unit tests we have for the types all pass.

Amos
-------------- next part --------------
=== modified file 'src/Makefile.am'
--- src/Makefile.am	2016-03-18 09:38:10 +0000
+++ src/Makefile.am	2016-04-12 12:09:40 +0000
@@ -357,8 +357,6 @@
 	ipcache.cc \
 	ipcache.h \
 	$(LEAKFINDERSOURCE) \
-	SquidList.h \
-	SquidList.cc \
 	LogTags.cc \
 	LogTags.h \
 	lookup_t.h \
@@ -1058,8 +1056,6 @@
 	MasterXaction.h \
 	Notes.cc \
 	Notes.h \
-	SquidList.h \
-	SquidList.cc \
 	mem_node.cc \
 	Parsing.cc \
 	tests/stub_libsecurity.cc \
@@ -1297,8 +1293,6 @@
 	internal.cc \
 	LogTags.cc \
 	tests/stub_libsecurity.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	multicast.h \
@@ -1479,8 +1473,6 @@
 	HttpReply.cc \
 	int.h \
 	int.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -1725,8 +1717,6 @@
 	internal.cc \
 	LogTags.cc \
 	tests/stub_libsecurity.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	tests/stub_libmem.cc \
@@ -1964,8 +1954,6 @@
 	internal.h \
 	internal.cc \
 	LogTags.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -2199,8 +2187,6 @@
 	$(IPC_SOURCE) \
 	ipcache.cc \
 	LogTags.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -2516,8 +2502,6 @@
 	internal.cc \
 	LogTags.cc \
 	tests/stub_libsecurity.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	multicast.h \
@@ -2730,8 +2714,6 @@
 	RequestFlags.h \
 	int.h \
 	int.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	mem_node.cc \
@@ -2956,8 +2938,6 @@
 	int.cc \
 	RequestFlags.h \
 	RequestFlags.cc \
-	SquidList.h \
-	SquidList.cc \
 	Transients.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
@@ -3138,8 +3118,6 @@
 	HttpReply.cc \
 	int.h \
 	int.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -3351,8 +3329,6 @@
 	internal.cc \
 	tests/stub_libeui.cc \
 	LogTags.cc \
-	SquidList.h \
-	SquidList.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	multicast.h \

=== removed file 'src/SquidList.cc'
--- src/SquidList.cc	2016-01-01 00:12:18 +0000
+++ src/SquidList.cc	1970-01-01 00:00:00 +0000
@@ -1,49 +0,0 @@
-/*
- * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-/* DEBUG: none          Linked list functions (deprecated) */
-
-#include "squid.h"
-#include "mem/forward.h"
-#include "SquidList.h"
-
-/* This should go away, in favour of the List template class */
-
-void
-linklistPush(link_list ** L, void *p)
-{
-    link_list *l = (link_list *)memAllocate(MEM_LINK_LIST);
-    l->next = NULL;
-    l->ptr = p;
-
-    while (*L)
-        L = &(*L)->next;
-
-    *L = l;
-}
-
-void *
-linklistShift(link_list ** L)
-{
-    void *p;
-    link_list *l;
-
-    if (NULL == *L)
-        return NULL;
-
-    l = *L;
-
-    p = l->ptr;
-
-    *L = (*L)->next;
-
-    memFree(l, MEM_LINK_LIST);
-
-    return p;
-}
-

=== removed file 'src/SquidList.h'
--- src/SquidList.h	2016-01-01 00:12:18 +0000
+++ src/SquidList.h	1970-01-01 00:00:00 +0000
@@ -1,25 +0,0 @@
-/*
- * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-/* DEBUG: none          Linked list functions (deprecated) */
-
-#ifndef SQUID_SQUIDLIST_H_
-#define SQUID_SQUIDLIST_H_
-
-class link_list
-{
-public:
-    void *ptr;
-    link_list *next;
-};
-
-void linklistPush(link_list **, void *);
-void *linklistShift(link_list **);
-
-#endif /* SQUID_SQUIDLIST_H_ */
-

=== modified file 'src/fs/ufs/UFSStoreState.cc'
--- src/fs/ufs/UFSStoreState.cc	2016-01-01 00:12:18 +0000
+++ src/fs/ufs/UFSStoreState.cc	2016-04-12 15:42:12 +0000
@@ -14,7 +14,6 @@
 #include "DiskIO/ReadRequest.h"
 #include "DiskIO/WriteRequest.h"
 #include "Generic.h"
-#include "SquidList.h"
 #include "Store.h"
 #include "store/Disk.h"
 #include "UFSStoreState.h"
@@ -181,17 +180,18 @@
 void
 Fs::Ufs::UFSStoreState::doWrite()
 {
-    debugs(79, 3, HERE << this << " UFSStoreState::doWrite");
-
+    debugs(79, 3, static_cast<void *>(this));
     assert(theFile->canWrite());
 
-    _queued_write *q = (_queued_write *)linklistShift(&pending_writes);
+    _queued_write *q = pending_writes.front();
 
-    if (q == NULL) {
+    if (!q) {
         debugs(79, 3, HERE << this << " UFSStoreState::doWrite queue is empty");
         return;
     }
 
+    pending_writes.pop();
+
     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);
@@ -328,8 +328,6 @@
     closing(false),
     reading(false),
     writing(false),
-    pending_reads(NULL),
-    pending_writes(NULL),
     read_buf(NULL)
 {
     // StoreIOState inherited members
@@ -344,55 +342,53 @@
 
 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))) {
+    while (auto *qr = pending_reads.front()) {
         cbdataReferenceDone(qr->callback_data);
+        pending_reads.pop();
         delete qr;
     }
 
-    debugs(79,3,HERE << "UFSStoreState::freePending: freed pending reads");
-
-    _queued_write *qw;
-
-    while ((qw = (_queued_write *)linklistShift(&pending_writes))) {
+    debugs(79, 3, "freed pending reads");
+
+    while (auto *qw = pending_writes.front()) {
         if (qw->free_func)
             qw->free_func(const_cast<char *>(qw->buf));
+        pending_writes.pop();
         delete qw;
     }
 
-    debugs(79,3,HERE << "UFSStoreState::freePending: freed pending writes");
+    debugs(79, 3, "freed pending writes");
 }
 
 bool
 Fs::Ufs::UFSStoreState::kickReadQueue()
 {
-    _queued_read *q = (_queued_read *)linklistShift(&pending_reads);
+    _queued_read *q = pending_reads.front();
 
-    if (NULL == q)
+    if (!q)
         return false;
 
-    debugs(79, 3, "UFSStoreState::kickReadQueue: reading queued request of " << q->size << " bytes");
+    debugs(79, 3, "reading queued request of " << q->size << " bytes");
+
+    pending_reads.pop();
 
     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);
+        debugs(79, 2, "this: " << this << " cbdataReferenceValidDone returned false. closing: " << closing << " flags.try_closing: " << flags.try_closing);
         delete q;
         return false;
     }
 
     delete q;
-
     return true;
 }
 
@@ -401,14 +397,14 @@
 {
     debugs(79, 3, "UFSStoreState::queueRead: queueing read");
     assert(opening);
-    assert (pending_reads == NULL);
+    assert(pending_reads.empty());
     _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.push(q);
 }
 
 /*
@@ -435,7 +431,7 @@
 
     flags.write_draining = true;
 
-    while (pending_writes != NULL) {
+    while (!pending_writes.empty()) {
         doWrite();
     }
 
@@ -475,14 +471,13 @@
 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);
+    debugs(79, 3, static_cast<void*>(this) << ": queueing write of size " << size);
 
-    _queued_write *q;
-    q = new _queued_write;
+    _queued_write *q = new _queued_write;
     q->buf = buf;
     q->size = size;
     q->offset = aOffset;
     q->free_func = free_func;
-    linklistPush(&pending_writes, q);
+    pending_writes.push(q);
 }
 

=== modified file 'src/fs/ufs/UFSStoreState.h'
--- src/fs/ufs/UFSStoreState.h	2016-01-01 00:12:18 +0000
+++ src/fs/ufs/UFSStoreState.h	2016-04-12 13:20:16 +0000
@@ -10,9 +10,10 @@
 #define SQUID_FS_UFS_UFSSTORESTATE_H
 
 #include "DiskIO/IORequestor.h"
-#include "SquidList.h"
 #include "StoreIOState.h"
 
+#include <queue>
+
 namespace Fs
 {
 namespace Ufs
@@ -98,8 +99,8 @@
          */
         bool try_closing;
     } flags;
-    link_list *pending_reads;
-    link_list *pending_writes;
+    std::queue<_queued_read *> pending_reads;
+    std::queue<_queued_write *> pending_writes;
     void queueRead(char *, size_t, off_t, STRCB *, void *);
     void queueWrite(char const *, size_t, off_t, FREE *);
     bool kickReadQueue();

=== modified file 'src/mem/forward.h'
--- src/mem/forward.h	2016-04-12 10:44:44 +0000
+++ src/mem/forward.h	2016-04-12 12:10:29 +0000
@@ -46,7 +46,6 @@
     MEM_64K_BUF,
     MEM_ACL_DENY_INFO_LIST,
     MEM_ACL_NAME_LIST,
-    MEM_LINK_LIST,
     MEM_DLINK_NODE,
     MEM_DREAD_CTRL,
     MEM_DWRITE_Q,

=== modified file 'src/mem/old_api.cc'
--- src/mem/old_api.cc	2016-04-12 10:44:44 +0000
+++ src/mem/old_api.cc	2016-04-12 12:10:32 +0000
@@ -24,7 +24,6 @@
 #include "MemBuf.h"
 #include "mgr/Registration.h"
 #include "SquidConfig.h"
-#include "SquidList.h"
 #include "SquidTime.h"
 #include "Store.h"
 
@@ -445,7 +444,6 @@
     memDataInit(MEM_ACL_DENY_INFO_LIST, "AclDenyInfoList",
                 sizeof(AclDenyInfoList), 0);
     memDataInit(MEM_ACL_NAME_LIST, "acl_name_list", sizeof(AclNameList), 0);
-    memDataInit(MEM_LINK_LIST, "link_list", sizeof(link_list), 10);
     memDataInit(MEM_DLINK_NODE, "dlink_node", sizeof(dlink_node), 10);
     memDataInit(MEM_DREAD_CTRL, "dread_ctrl", sizeof(dread_ctrl), 0);
     memDataInit(MEM_DWRITE_Q, "dwrite_q", sizeof(dwrite_q), 0);

=== modified file 'src/repl/heap/store_repl_heap.cc'
--- src/repl/heap/store_repl_heap.cc	2016-01-01 00:12:18 +0000
+++ src/repl/heap/store_repl_heap.cc	2016-04-13 01:14:51 +0000
@@ -20,7 +20,6 @@
 #include "squid.h"
 #include "heap.h"
 #include "MemObject.h"
-#include "SquidList.h"
 #include "Store.h"
 #include "store_heap_replacement.h"
 #include "wordlist.h"
@@ -181,10 +180,12 @@
 
 /** RemovalPurgeWalker **/
 
-typedef struct _HeapPurgeData HeapPurgeData;
+class HeapPurgeData
+{
+public:
+    HeapPurgeData() : min_age(0.0) {}
 
-struct _HeapPurgeData {
-    link_list *locked_entries;
+    std::list<StoreEntry *> locked_entries;
     heap_key min_age;
 };
 
@@ -197,22 +198,18 @@
     StoreEntry *entry;
     heap_key age;
 
-try_again:
-
-    if (heap_empty(h->theHeap))
-        return NULL;        /* done */
-
-    age = heap_peepminkey(h->theHeap);
-
-    entry = (StoreEntry *)heap_extractmin(h->theHeap);
-
-    if (entry->locked()) {
-
-        entry->lock("heap_purgeNext");
-        linklistPush(&heap_walker->locked_entries, entry);
-
-        goto try_again;
-    }
+    do {
+        if (heap_empty(h->theHeap))
+            return nullptr;
+
+        age = heap_peepminkey(h->theHeap);
+        entry = (StoreEntry *)heap_extractmin(h->theHeap);
+
+        if (entry->locked()) {
+            entry->lock("heap_purgeNext");
+            heap_walker->locked_entries.push_back(entry);
+        }
+    } while (entry->locked());
 
     heap_walker->min_age = age;
     h->setPolicyNode(entry, NULL);
@@ -225,7 +222,6 @@
     HeapPurgeData *heap_walker = (HeapPurgeData *)walker->_data;
     RemovalPolicy *policy = walker->_policy;
     HeapPolicyData *h = (HeapPolicyData *)policy->_data;
-    StoreEntry *entry;
     assert(strcmp(policy->_type, "heap") == 0);
     assert(h->nwalkers > 0);
     h->nwalkers -= 1;
@@ -235,13 +231,12 @@
         debugs(81, 3, "Heap age set to " << h->theHeap->age);
     }
 
-    /*
-     * Reinsert the locked entries
-     */
-    while ((entry = (StoreEntry *)linklistShift(&heap_walker->locked_entries))) {
+    // Reinsert the locked entries
+    while (StoreEntry *entry = heap_walker->locked_entries.front()) {
         heap_node *node = heap_insert(h->theHeap, entry);
         h->setPolicyNode(entry, node);
         entry->unlock("heap_purgeDone");
+        heap_walker->locked_entries.pop_front();
     }
 
     safe_free(walker->_data);
@@ -253,14 +248,10 @@
 {
     HeapPolicyData *h = (HeapPolicyData *)policy->_data;
     RemovalPurgeWalker *walker;
-    HeapPurgeData *heap_walk;
     h->nwalkers += 1;
     walker = new RemovalPurgeWalker;
-    heap_walk = (HeapPurgeData *)xcalloc(1, sizeof(*heap_walk));
-    heap_walk->min_age = 0.0;
-    heap_walk->locked_entries = NULL;
     walker->_policy = policy;
-    walker->_data = heap_walk;
+    walker->_data = new HeapPurgeData;
     walker->max_scan = max_scan;
     walker->Next = heap_purgeNext;
     walker->Done = heap_purgeDone;



More information about the squid-dev mailing list