From a578862a32c2cb7c2cd3f4dc3395feddc701d282 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 16 Oct 2012 18:02:26 -0600 Subject: [PATCH] Reverted trunk r12255 changes. Provided a portable flexible arrays replacement. Trunk r12255 made Clang compiler happy by removing flexible nonPod[] arrays. Unfortunately, it also moved shared memory items into local memory (in some cases uninitialized). This change provides a Clang-friendly flexible array replacement while keeping items in the shared memory (and using placement-new initialization). The code may have become even less readable, but hopefully more portable. N.B. Flexible arrays were introdiced in C99 standard, after C++ was standardized in 1998. They are not yet in any revised C++ standard, but most C++ compilers support them, at least for PODs. --- src/ipc/Makefile.am | 1 + src/ipc/Queue.cc | 9 ++------ src/ipc/Queue.h | 8 ++----- src/ipc/StoreMap.cc | 12 +++------- src/ipc/StoreMap.h | 8 ++----- src/ipc/mem/FlexibleArray.h | 45 +++++++++++++++++++++++++++++++++++++ src/ipc/mem/PageStack.cc | 9 ++------ src/ipc/mem/PageStack.h | 4 ++-- 8 files changed, 59 insertions(+), 37 deletions(-) create mode 100644 src/ipc/mem/FlexibleArray.h diff --git a/src/ipc/Makefile.am b/src/ipc/Makefile.am index 9e2f7dcfda..3a08d6ea46 100644 --- a/src/ipc/Makefile.am +++ b/src/ipc/Makefile.am @@ -46,6 +46,7 @@ libipc_la_SOURCES = \ Request.h \ Response.h \ \ + mem/FlexibleArray.h \ mem/Page.cc \ mem/Page.h \ mem/PagePool.cc \ diff --git a/src/ipc/Queue.cc b/src/ipc/Queue.cc index 9ce1b94e83..aa977b0350 100644 --- a/src/ipc/Queue.cc +++ b/src/ipc/Queue.cc @@ -48,15 +48,10 @@ Ipc::QueueReader::QueueReader(): popBlocked(1), popSignal(0), /* QueueReaders */ -Ipc::QueueReaders::QueueReaders(const int aCapacity): theCapacity(aCapacity) +Ipc::QueueReaders::QueueReaders(const int aCapacity): theCapacity(aCapacity), + theReaders(theCapacity) { Must(theCapacity > 0); - theReaders=new QueueReader[theCapacity]; -} - -Ipc::QueueReaders::~QueueReaders() -{ - delete[] theReaders; } size_t diff --git a/src/ipc/Queue.h b/src/ipc/Queue.h index 45fb3af576..f96099611f 100644 --- a/src/ipc/Queue.h +++ b/src/ipc/Queue.h @@ -10,6 +10,7 @@ #include "Debug.h" #include "base/InstanceId.h" #include "ipc/AtomicWord.h" +#include "ipc/mem/FlexibleArray.h" #include "ipc/mem/Pointer.h" #include "util.h" @@ -64,16 +65,11 @@ class QueueReaders { public: QueueReaders(const int aCapacity); - ~QueueReaders(); size_t sharedMemorySize() const; static size_t SharedMemorySize(const int capacity); const int theCapacity; /// number of readers - QueueReader *theReaders; /// readers -private: - QueueReaders(); //not implemented - QueueReaders& operator =(const QueueReaders&); //not implemented - QueueReaders(const QueueReaders&); //not implemented + Ipc::Mem::FlexibleArray theReaders; /// readers }; /** diff --git a/src/ipc/StoreMap.cc b/src/ipc/StoreMap.cc index 86bc941565..537c6c9a34 100644 --- a/src/ipc/StoreMap.cc +++ b/src/ipc/StoreMap.cc @@ -256,14 +256,14 @@ void Ipc::StoreMap::freeLocked(Slot &s, bool keepLocked) { if (s.state == Slot::Readable && cleaner) - cleaner->cleanReadable(&s - shared->slots); + cleaner->cleanReadable(&s - shared->slots.raw()); s.waitingToBeFreed = false; s.state = Slot::Empty; if (!keepLocked) s.lock.unlockExclusive(); --shared->count; - debugs(54, 5, HERE << " freed slot at " << (&s - shared->slots) << + debugs(54, 5, HERE << " freed slot at " << (&s - shared->slots.raw()) << " in map [" << path << ']'); } @@ -306,14 +306,8 @@ Ipc::StoreMapSlot::set(const StoreEntry &from) /* Ipc::StoreMap::Shared */ Ipc::StoreMap::Shared::Shared(const int aLimit, const size_t anExtrasSize): - limit(aLimit), extrasSize(anExtrasSize), count(0) + limit(aLimit), extrasSize(anExtrasSize), count(0), slots(aLimit) { - slots=new Slot[limit]; -} - -Ipc::StoreMap::Shared::~Shared() -{ - delete[] slots; } size_t diff --git a/src/ipc/StoreMap.h b/src/ipc/StoreMap.h index 7006b39cb1..10ebed5c45 100644 --- a/src/ipc/StoreMap.h +++ b/src/ipc/StoreMap.h @@ -2,6 +2,7 @@ #define SQUID_IPC_STORE_MAP_H #include "ipc/ReadWriteLock.h" +#include "ipc/mem/FlexibleArray.h" #include "ipc/mem/Pointer.h" #include "typedefs.h" @@ -62,16 +63,11 @@ public: Shared(const int aLimit, const size_t anExtrasSize); size_t sharedMemorySize() const; static size_t SharedMemorySize(const int limit, const size_t anExtrasSize); - ~Shared(); const int limit; ///< maximum number of map slots const size_t extrasSize; ///< size of slot extra data Atomic::Word count; ///< current number of map slots - Slot *slots; ///< slots storage - private: - Shared(); //disabled - Shared &operator=(const Shared&); //disabled - Shared(const Shared&); //disabled + Ipc::Mem::FlexibleArray slots; ///< slots storage }; public: diff --git a/src/ipc/mem/FlexibleArray.h b/src/ipc/mem/FlexibleArray.h new file mode 100644 index 0000000000..0af9971f06 --- /dev/null +++ b/src/ipc/mem/FlexibleArray.h @@ -0,0 +1,45 @@ +/* + */ + +#ifndef SQUID_IPC_MEM_FLEXIBLE_ARRAY_H +#define SQUID_IPC_MEM_FLEXIBLE_ARRAY_H + +// sometimes required for placement-new operator to be declared +#include + +namespace Ipc +{ + +namespace Mem +{ + +/// A "flexible array" of Items inside some shared memory space. +/// A portable equivalent of a "Item items[];" data member. +/// Some compilers such as Clang can only handle flexible arrays of PODs, +/// and the current C++ standard does not allow flexible arrays at all. +template +class FlexibleArray +{ +public: + explicit FlexibleArray(const int capacity) { + if (capacity > 1) // the first item is initialized automatically + new (items+1) Item[capacity-1]; + } + + Item &operator [](const int idx) { return items[idx]; } + const Item &operator [](const int idx) const { return items[idx]; } + + //const Item *operator ()() const { return items; } + //Item *operator ()() { return items; } + + Item *raw() { return items; } + +private: + Item items[1]; // ensures proper alignment of array elements +}; + +} // namespace Mem + +} // namespace Ipc + +#endif /* SQUID_IPC_MEM_FLEXIBLE_ARRAY_H */ diff --git a/src/ipc/mem/PageStack.cc b/src/ipc/mem/PageStack.cc index fea71d272b..0809e06808 100644 --- a/src/ipc/mem/PageStack.cc +++ b/src/ipc/mem/PageStack.cc @@ -18,19 +18,14 @@ const Ipc::Mem::PageStack::Value Writable = 0; Ipc::Mem::PageStack::PageStack(const uint32_t aPoolId, const unsigned int aCapacity, const size_t aPageSize): thePoolId(aPoolId), theCapacity(aCapacity), thePageSize(aPageSize), theSize(theCapacity), - theLastReadable(prev(theSize)), theFirstWritable(next(theLastReadable)) + theLastReadable(prev(theSize)), theFirstWritable(next(theLastReadable)), + theItems(aCapacity) { - theItems=new Item[theSize]; // initially, all pages are free for (Offset i = 0; i < theSize; ++i) theItems[i] = i + 1; // skip page number zero to keep numbers positive } -Ipc::Mem::PageStack::~PageStack() -{ - delete[] theItems; -} - /* * TODO: We currently rely on the theLastReadable hint during each * loop iteration. We could also use hint just for the start position: diff --git a/src/ipc/mem/PageStack.h b/src/ipc/mem/PageStack.h index 85aea0901d..47763ec3d9 100644 --- a/src/ipc/mem/PageStack.h +++ b/src/ipc/mem/PageStack.h @@ -7,6 +7,7 @@ #define SQUID_IPC_MEM_PAGE_STACK_H #include "ipc/AtomicWord.h" +#include "ipc/mem/FlexibleArray.h" namespace Ipc { @@ -25,7 +26,6 @@ public: typedef uint32_t Value; ///< stack item type (a free page number) PageStack(const uint32_t aPoolId, const unsigned int aCapacity, const size_t aPageSize); - ~PageStack(); unsigned int capacity() const { return theCapacity; } size_t pageSize() const { return thePageSize; } @@ -68,7 +68,7 @@ private: Atomic::WordT theFirstWritable; typedef Atomic::WordT Item; - Item *theItems; ///< page number storage + Ipc::Mem::FlexibleArray theItems; ///< page number storage }; } // namespace Mem -- 2.47.2