------------------------------------------------------------ revno: 12366 revision-id: squid3@treenet.co.nz-20121016234607-l545ojr7jj6v6h80 parent: squid3@treenet.co.nz-20121016234354-zo8ojneglp4t3bj0 author: Alex Rousskov committer: Amos Jeffries branch nick: 3.3 timestamp: Tue 2012-10-16 17:46:07 -0600 message: 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. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20121016234607-l545ojr7jj6v6h80 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # testament_sha1: df84435df0663542f1df73c1d5cac729becd2604 # timestamp: 2012-10-16 23:47:07 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # base_revision_id: squid3@treenet.co.nz-20121016234354-\ # zo8ojneglp4t3bj0 # # Begin patch === modified file 'src/ipc/Makefile.am' --- src/ipc/Makefile.am 2011-10-28 01:01:41 +0000 +++ src/ipc/Makefile.am 2012-10-16 23:46:07 +0000 @@ -46,6 +46,7 @@ Request.h \ Response.h \ \ + mem/FlexibleArray.h \ mem/Page.cc \ mem/Page.h \ mem/PagePool.cc \ === modified file 'src/ipc/Queue.cc' --- src/ipc/Queue.cc 2012-09-01 14:38:36 +0000 +++ src/ipc/Queue.cc 2012-10-16 23:46:07 +0000 @@ -45,15 +45,10 @@ /* 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 === modified file 'src/ipc/Queue.h' --- src/ipc/Queue.h 2012-09-01 14:38:36 +0000 +++ src/ipc/Queue.h 2012-10-16 23:46:07 +0000 @@ -8,6 +8,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" @@ -62,16 +63,11 @@ { 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 }; /** === modified file 'src/ipc/StoreMap.cc' --- src/ipc/StoreMap.cc 2012-08-31 16:57:39 +0000 +++ src/ipc/StoreMap.cc 2012-10-16 23:46:07 +0000 @@ -255,14 +255,14 @@ 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 << ']'); } @@ -304,14 +304,8 @@ /* Ipc::StoreMap::Shared */ Ipc::StoreMap::Shared::Shared(const int aLimit, const size_t anExtrasSize): - limit(aLimit), extrasSize(anExtrasSize), count(0) -{ - slots=new Slot[limit]; -} - -Ipc::StoreMap::Shared::~Shared() -{ - delete[] slots; + limit(aLimit), extrasSize(anExtrasSize), count(0), slots(aLimit) +{ } size_t === modified file 'src/ipc/StoreMap.h' --- src/ipc/StoreMap.h 2012-08-28 13:00:30 +0000 +++ src/ipc/StoreMap.h 2012-10-16 23:46:07 +0000 @@ -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 @@ 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: === added file 'src/ipc/mem/FlexibleArray.h' --- src/ipc/mem/FlexibleArray.h 1970-01-01 00:00:00 +0000 +++ src/ipc/mem/FlexibleArray.h 2012-10-16 23:46:07 +0000 @@ -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 */ === modified file 'src/ipc/mem/PageStack.cc' --- src/ipc/mem/PageStack.cc 2012-10-03 10:18:42 +0000 +++ src/ipc/mem/PageStack.cc 2012-10-16 23:46:07 +0000 @@ -15,19 +15,14 @@ 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: === modified file 'src/ipc/mem/PageStack.h' --- src/ipc/mem/PageStack.h 2012-09-01 14:38:36 +0000 +++ src/ipc/mem/PageStack.h 2012-10-16 23:46:07 +0000 @@ -5,6 +5,7 @@ #define SQUID_IPC_MEM_PAGE_STACK_H #include "ipc/AtomicWord.h" +#include "ipc/mem/FlexibleArray.h" namespace Ipc { @@ -23,7 +24,6 @@ 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; } @@ -66,7 +66,7 @@ Atomic::WordT theFirstWritable; typedef Atomic::WordT Item; - Item *theItems; ///< page number storage + Ipc::Mem::FlexibleArray theItems; ///< page number storage }; } // namespace Mem