BPackageInfoSet: Add copy constructor, assignment operator

* Implement copy-on-write support.
* Add copy constructor and assignment operator.
* Remove Init(). Initialize lazily instead. Since AddInfo() can fail
  and we check initialization anyway, there's no point in having an
  explicit Init(). Given that there was only one invocation of Init()
  in the package kit and its users, it was very likely missing in some
  places.
* Fix a few places where we ignored that the PackageMap actually
  contains lists of PackageInfo objects.

headers/os/package/PackageInfoSet.h

@@ -21,16 +21,20 @@ public:
 
 public:
 								BPackageInfoSet();
+								BPackageInfoSet(const BPackageInfoSet& other);
 	virtual						~BPackageInfoSet();
 
-			status_t			Init();
-
 			status_t			AddInfo(const BPackageInfo& info);
 			void				MakeEmpty();
 
 			uint32				CountInfos() const;
 			Iterator			GetIterator() const;
 
+			BPackageInfoSet&	operator=(const BPackageInfoSet& other);
+
+private:
+			bool				_CopyOnWrite();
+
 private:
 			struct PackageInfo;
 			struct PackageInfoHashDefinition;
@@ -46,7 +50,7 @@ private:
 class BPackageInfoSet::Iterator {
 public:
 								Iterator();
-								Iterator(const BPackageInfoSet* set);
+								Iterator(const PackageMap* map);
 
 			bool				HasNext() const;
 			const BPackageInfo*	Next();
@@ -55,7 +59,7 @@ private:
 			friend class BRepositoryCache;
 
 private:
-			const BPackageInfoSet* fSet;
+			const PackageMap*	fMap;
 			PackageInfo*		fNextInfo;
 };
 

src/kits/package/PackageInfoSet.cpp

@@ -12,6 +12,10 @@
 
 #include <new>
 
+#include <Referenceable.h>
+
+#include <AutoDeleter.h>
+
 #include <util/OpenHashTable.h>
 
 #include <package/PackageInfo.h>
@@ -33,6 +37,16 @@ struct BPackageInfoSet::PackageInfo : public BPackageInfo {
 		listNext(NULL)
 	{
 	}
+
+	void DeleteList()
+	{
+		PackageInfo* info = this;
+		while (info != NULL) {
+			PackageInfo* next = info->listNext;
+			delete info;
+			info = next;
+		}
+	}
 };
 
 
@@ -68,8 +82,8 @@ struct BPackageInfoSet::PackageInfoHashDefinition {
 // #pragma mark - PackageMap
 
 
-struct BPackageInfoSet::PackageMap
-	: public BOpenHashTable<PackageInfoHashDefinition> {
+struct BPackageInfoSet::PackageMap : public BReferenceable,
+	public BOpenHashTable<PackageInfoHashDefinition> {
 
 	PackageMap()
 		:
@@ -79,12 +93,34 @@ struct BPackageInfoSet::PackageMap
 
 	~PackageMap()
 	{
-		PackageInfo* info = Clear(true);
-		while (info != NULL) {
-			PackageInfo* next = info->hashNext;
-			delete info;
-			info = next;
+		DeleteAllPackageInfos();
+	}
+
+	static PackageMap* Create()
+	{
+		PackageMap* map = new(std::nothrow) PackageMap;
+		if (map == NULL || map->Init() != B_OK) {
+			delete map;
+			return NULL;
 		}
+
+		return map;
+	}
+
+	PackageMap* Clone() const
+	{
+		PackageMap* newMap = Create();
+		if (newMap == NULL)
+			return NULL;
+		ObjectDeleter<PackageMap> newMapDeleter(newMap);
+
+		for (BPackageInfoSet::Iterator it(this); it.HasNext();) {
+			const BPackageInfo* info = it.Next();
+			if (newMap->AddNewPackageInfo(*info) != B_OK)
+				return NULL;
+		}
+
+		return newMapDeleter.Detach();
 	}
 
 	void AddPackageInfo(PackageInfo* info)
@@ -98,6 +134,32 @@ struct BPackageInfoSet::PackageMap
 		fCount++;
 	}
 
+	status_t AddNewPackageInfo(const BPackageInfo& oldInfo)
+	{
+		PackageInfo* info = new(std::nothrow) PackageInfo(oldInfo);
+		if (info == NULL)
+			return B_NO_MEMORY;
+		ObjectDeleter<PackageInfo> infoDeleter(info);
+	
+		status_t error = info->InitCheck();
+		if (error != B_OK)
+			return error;
+	
+		AddPackageInfo(infoDeleter.Detach());
+	
+		return B_OK;
+	}
+
+	void DeleteAllPackageInfos()
+	{
+		PackageInfo* info = Clear(true);
+		while (info != NULL) {
+			PackageInfo* next = info->hashNext;
+			info->DeleteList();
+			info = next;
+		}
+	}
+
 	uint32 CountPackageInfos() const
 	{
 		return fCount;
@@ -113,19 +175,20 @@ private:
 
 BPackageInfoSet::Iterator::Iterator()
 	:
-	fSet(NULL),
+	fMap(NULL),
 	fNextInfo(NULL)
 {
 }
 
 
-BPackageInfoSet::Iterator::Iterator(const BPackageInfoSet* set)
+BPackageInfoSet::Iterator::Iterator(const PackageMap* map)
 	:
-	fSet(set),
-	fNextInfo(fSet->fPackageMap->GetIterator().Next())
+	fMap(map),
+	fNextInfo(map->GetIterator().Next())
 {
 }
 
+
 bool
 BPackageInfoSet::Iterator::HasNext() const
 {
@@ -145,7 +208,7 @@ BPackageInfoSet::Iterator::Next()
 		} else {
 			// get next in hash table
 			PackageMap::Iterator iterator
-				= fSet->fPackageMap->GetIterator(fNextInfo->Name());
+				= fMap->GetIterator(fNextInfo->Name());
 			iterator.Next();
 			fNextInfo = iterator.Next();
 		}
@@ -160,64 +223,52 @@ BPackageInfoSet::Iterator::Next()
 
 BPackageInfoSet::BPackageInfoSet()
 	:
-	fPackageMap(new(std::nothrow) PackageMap)
+	fPackageMap(NULL)
 {
 }
 
 
 BPackageInfoSet::~BPackageInfoSet()
 {
-	MakeEmpty();
-	delete fPackageMap;
+	if (fPackageMap != NULL)
+		fPackageMap->ReleaseReference();
+}
+
+
+BPackageInfoSet::BPackageInfoSet(const BPackageInfoSet& other)
+	:
+	fPackageMap(other.fPackageMap)
+{
+	if (fPackageMap != NULL)
+		fPackageMap->AcquireReference();
 }
 
 
 status_t
-BPackageInfoSet::Init()
+BPackageInfoSet::AddInfo(const BPackageInfo& info)
 {
-	return fPackageMap->Init();
-}
-
-
-status_t
-BPackageInfoSet::AddInfo(const BPackageInfo& _info)
-{
-	if (fPackageMap == NULL)
-		return B_NO_INIT;
-
-	PackageInfo* info = new(std::nothrow) PackageInfo(_info);
-	if (info == NULL)
+	if (!_CopyOnWrite())
 		return B_NO_MEMORY;
 
-	status_t error = info->InitCheck();
-	if (error != B_OK) {
-		delete info;
-		return error;
-	}
-
-	if (PackageInfo* oldInfo = fPackageMap->Lookup(info->Name())) {
-		// TODO: Check duplicates?
-		info->listNext = oldInfo->listNext;
-		oldInfo->listNext = info;
-	} else
-		fPackageMap->Insert(info);
-
-	return B_OK;
+	return fPackageMap->AddNewPackageInfo(info);
 }
 
 
 void
 BPackageInfoSet::MakeEmpty()
 {
-	if (fPackageMap == NULL)
+	if (fPackageMap == NULL || fPackageMap->CountPackageInfos() == 0)
 		return;
 
-	PackageInfo* info = fPackageMap->Clear(true);
-	while (info != NULL) {
-		PackageInfo* next = info->hashNext;
-		delete info;
-		info = next;
+	// If our map is shared, just set it to NULL.
+	if (fPackageMap->CountReferences() != 1) {
+		fPackageMap->ReleaseReference();
+		fPackageMap = NULL;
+		return;
 	}
+
+	// Our map is not shared -- make it empty.
+	fPackageMap->DeleteAllPackageInfos();
 }
 
 
@@ -234,7 +285,46 @@ BPackageInfoSet::CountInfos() const
 BPackageInfoSet::Iterator
 BPackageInfoSet::GetIterator() const
 {
-	return Iterator(this);
+	return Iterator(fPackageMap);
+}
+
+
+BPackageInfoSet&
+BPackageInfoSet::operator=(const BPackageInfoSet& other)
+{
+	if (other.fPackageMap == fPackageMap)
+		return *this;
+
+	if (fPackageMap != NULL)
+		fPackageMap->ReleaseReference();
+
+	fPackageMap = other.fPackageMap;
+
+	if (fPackageMap != NULL)
+		fPackageMap->AcquireReference();
+
+	return *this;
+}
+
+
+bool
+BPackageInfoSet::_CopyOnWrite()
+{
+	if (fPackageMap == NULL) {
+		fPackageMap = PackageMap::Create();
+		return fPackageMap != NULL;
+	}
+
+	if (fPackageMap->CountReferences() == 1)
+		return true;
+
+	PackageMap* newMap = fPackageMap->Clone();
+	if (newMap == NULL)
+		return false;
+
+	fPackageMap->ReleaseReference();
+	fPackageMap = newMap;
+	return true;
 }
 
 

src/kits/package/RepositoryCache.cpp

@@ -243,15 +243,11 @@ BRepositoryCache::SetTo(const BEntry& entry)
 	fPackages.MakeEmpty();
 	fEntry.Unset();
 
-	// init package info set
-	status_t result = fPackages.Init();
-	if (result != B_OK)
-		return result;
-
 	// get cache file path
 	fEntry = entry;
 
 	BPath repositoryCachePath;
+	status_t result;
 	if ((result = entry.GetPath(&repositoryCachePath)) != B_OK)
 		return result;