diff --git a/headers/private/package/packagefs.h b/headers/private/package/packagefs.h index d7ef1a9dc0..8768e6c0ef 100644 --- a/headers/private/package/packagefs.h +++ b/headers/private/package/packagefs.h @@ -79,7 +79,9 @@ struct PackageFSActivationChangeItem { uint32 nameLength; dev_t parentDeviceID; ino_t parentDirectoryID; - char name[1]; + char* name; + // must point to a location within the + // request }; struct PackageFSActivationChangeRequest { diff --git a/src/add-ons/kernel/file_systems/packagefs/Volume.cpp b/src/add-ons/kernel/file_systems/packagefs/Volume.cpp index f31fb43e7d..16126af844 100644 --- a/src/add-ons/kernel/file_systems/packagefs/Volume.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/Volume.cpp @@ -199,49 +199,6 @@ private: struct Volume::ActivationChangeRequest { -public: - struct Iterator { - Iterator() - : - fRequest(NULL), - fNextItem(NULL), - fNextIndex(0) - { - } - - Iterator(PackageFSActivationChangeRequest* request) - : - fRequest(request), - fNextItem(NULL), - fNextIndex(0) - { - if (fRequest != NULL && fRequest->itemCount > 0) - fNextItem = fRequest->items; - } - - bool HasNext() const - { - return fNextItem != NULL; - } - - PackageFSActivationChangeItem* Next() - { - if (fNextItem == NULL) - return NULL; - - PackageFSActivationChangeItem* item = fNextItem; - fNextIndex++; - fNextItem = fNextIndex < fRequest->itemCount - ? _NextItem(fNextItem) : NULL; - return item; - } - - private: - PackageFSActivationChangeRequest* fRequest; - PackageFSActivationChangeItem* fNextItem; - uint32 fNextIndex; - }; - public: ActivationChangeRequest() : @@ -270,22 +227,21 @@ public: if (error != B_OK) RETURN_ERROR(error); - // check the validity of the items - addr_t requestEnd = (addr_t)fRequest + fRequestSize; - uint32 itemCount = 0; - PackageFSActivationChangeItem* item = fRequest->items; - while (itemCount < fRequest->itemCount) { - if ((addr_t)item + sizeof(PackageFSActivationChangeItem) - > requestEnd - || item->nameLength > B_FILE_NAME_LENGTH - || (addr_t)item->name + item->nameLength > requestEnd - || item->name[item->nameLength] != '\0' - || strlen(item->name) != item->nameLength) { - RETURN_ERROR(B_BAD_VALUE); - } + uint32 itemCount = fRequest->itemCount; + const char* requestEnd = (const char*)fRequest + requestSize; + if (&fRequest->items[itemCount] > (void*)requestEnd) + RETURN_ERROR(B_BAD_VALUE); - itemCount++; - item = _NextItem(item); + // adjust the item name pointers and check their validity + addr_t nameDelta = (addr_t)fRequest - (addr_t)userRequest; + for (uint32 i = 0; i < itemCount; i++) { + PackageFSActivationChangeItem& item = fRequest->items[i]; + item.name += nameDelta; + if (item.name < (char*)fRequest || item.name >= requestEnd) + RETURN_ERROR(B_BAD_VALUE); + size_t maxNameSize = requestEnd - item.name; + if (strnlen(item.name, maxNameSize) == maxNameSize) + RETURN_ERROR(B_BAD_VALUE); } return B_OK; @@ -296,21 +252,9 @@ public: return fRequest->itemCount; } - Iterator GetIterator() const + PackageFSActivationChangeItem* ItemAt(uint32 index) const { - return Iterator(fRequest); - } - -private: - friend class Iterator; - // for GCC 2 - -private: - static inline PackageFSActivationChangeItem* _NextItem( - PackageFSActivationChangeItem* item) - { - return (PackageFSActivationChangeItem*)_ALIGN( - (addr_t)item->name + item->nameLength); + return index < CountItems() ? &fRequest->items[index] : NULL; } private: @@ -1362,7 +1306,8 @@ Volume::_LoadPackage(const char* name, Package*& _package) status_t Volume::_ChangeActivation(ActivationChangeRequest& request) { - if (request.CountItems() == 0) + uint32 itemCount = request.CountItems(); + if (itemCount == 0) return B_OK; // first check the request @@ -1371,9 +1316,8 @@ Volume::_ChangeActivation(ActivationChangeRequest& request) { VolumeReadLocker volumeLocker(this); - for (ActivationChangeRequest::Iterator it = request.GetIterator(); - it.HasNext();) { - PackageFSActivationChangeItem* item = it.Next(); + for (uint32 i = 0; i < itemCount; i++) { + PackageFSActivationChangeItem* item = request.ItemAt(i); if (item->parentDeviceID != fPackagesDirectory->DeviceID() || item->parentDirectoryID != fPackagesDirectory->NodeID()) { ERROR("Volume::_ChangeActivation(): mismatching packages " @@ -1429,9 +1373,8 @@ INFORM("Volume::_ChangeActivation(): %" B_PRId32 " new packages, %" B_PRId32 " o // load all new packages int32 newPackageIndex = 0; - for (ActivationChangeRequest::Iterator it = request.GetIterator(); - it.HasNext();) { - PackageFSActivationChangeItem* item = it.Next(); + for (uint32 i = 0; i < itemCount; i++) { + PackageFSActivationChangeItem* item = request.ItemAt(i); if (item->type != PACKAGE_FS_ACTIVATE_PACKAGE && item->type != PACKAGE_FS_REACTIVATE_PACKAGE) { @@ -1457,9 +1400,8 @@ INFORM("Volume::_ChangeActivation(): %" B_PRId32 " new packages, %" B_PRId32 " o // remove the old packages int32 oldPackageIndex = 0; - for (ActivationChangeRequest::Iterator it = request.GetIterator(); - it.HasNext();) { - PackageFSActivationChangeItem* item = it.Next(); + for (uint32 i = 0; i < itemCount; i++) { + PackageFSActivationChangeItem* item = request.ItemAt(i); if (item->type != PACKAGE_FS_DEACTIVATE_PACKAGE && item->type != PACKAGE_FS_REACTIVATE_PACKAGE) { diff --git a/src/servers/package/Root.cpp b/src/servers/package/Root.cpp index 6c2f505531..7a138789a5 100644 --- a/src/servers/package/Root.cpp +++ b/src/servers/package/Root.cpp @@ -220,6 +220,9 @@ void Root::_ProcessNodeMonitorEvents(Volume* volume) { volume->ProcessPendingNodeMonitorEvents(); + + if (volume->HasPendingPackageActivationChanges()) + volume->ProcessPendingPackageActivationChanges(); } diff --git a/src/servers/package/Volume.cpp b/src/servers/package/Volume.cpp index 014540d64f..b1e1d6a74a 100644 --- a/src/servers/package/Volume.cpp +++ b/src/servers/package/Volume.cpp @@ -81,7 +81,9 @@ Volume::Volume(BLooper* looper) fPackagesByFileName(), fPackagesByNodeRef(), fPendingNodeMonitorEventsLock("pending node monitor events"), - fPendingNodeMonitorEvents() + fPendingNodeMonitorEvents(), + fPackagesToBeActivated(), + fPackagesToBeDeactivated() { looper->AddHandler(this); } @@ -275,7 +277,6 @@ Volume::ProcessPendingNodeMonitorEvents() } // process them -// TODO: Don't do that individually. while (NodeMonitorEvent* event = events.RemoveHead()) { ObjectDeleter eventDeleter(event); if (event->WasCreated()) @@ -286,6 +287,100 @@ Volume::ProcessPendingNodeMonitorEvents() } +bool +Volume::HasPendingPackageActivationChanges() const +{ + return !fPackagesToBeActivated.empty() || !fPackagesToBeDeactivated.empty(); +} + + +void +Volume::ProcessPendingPackageActivationChanges() +{ + if (!HasPendingPackageActivationChanges()) + return; +INFORM("Volume::ProcessPendingPackageActivationChanges(): activating %zu, deactivating %zu packages\n", +fPackagesToBeActivated.size(), fPackagesToBeDeactivated.size()); + + // compute the size of the allocation we need for the activation change + // request + int32 itemCount + = fPackagesToBeActivated.size() + fPackagesToBeDeactivated.size(); + size_t requestSize = sizeof(PackageFSActivationChangeRequest) + + itemCount * sizeof(PackageFSActivationChangeItem); + + for (PackageSet::iterator it = fPackagesToBeActivated.begin(); + it != fPackagesToBeActivated.end(); ++it) { + requestSize += (*it)->FileName().Length() + 1; + } + + for (PackageSet::iterator it = fPackagesToBeDeactivated.begin(); + it != fPackagesToBeDeactivated.end(); ++it) { + requestSize += (*it)->FileName().Length() + 1; + } + + // allocate and prepare the request + PackageFSActivationChangeRequest* request + = (PackageFSActivationChangeRequest*)malloc(requestSize); + if (request == NULL) { + ERROR("out of memory\n"); + return; + } + MemoryDeleter requestDeleter(request); + + request->itemCount = itemCount; + + PackageFSActivationChangeItem* item = &request->items[0]; + char* nameBuffer = (char*)(item + itemCount); + + for (PackageSet::iterator it = fPackagesToBeActivated.begin(); + it != fPackagesToBeActivated.end(); ++it, item++) { + _FillInActivationChangeItem(item, PACKAGE_FS_ACTIVATE_PACKAGE, *it, + nameBuffer); + } + + for (PackageSet::iterator it = fPackagesToBeDeactivated.begin(); + it != fPackagesToBeDeactivated.end(); ++it, item++) { + _FillInActivationChangeItem(item, PACKAGE_FS_DEACTIVATE_PACKAGE, *it, + nameBuffer); + } + + // issue the request + int fd = OpenRootDirectory(); + if (fd < 0) { + ERROR("Volume::ProcessPendingPackageActivationChanges(): failed to " + "open root directory: %s", strerror(fd)); + return; + } + FileDescriptorCloser fdCloser(fd); + + if (ioctl(fd, PACKAGE_FS_OPERATION_CHANGE_ACTIVATION, request, requestSize) + != 0) { +// TODO: We need more error information and error handling! + ERROR("Volume::ProcessPendingPackageActivationChanges(): failed to " + "activate packages: %s\n", strerror(errno)); + return; + } + + // Update our state, i.e. remove deactivated packages and mark activated + // packages accordingly. + for (PackageSet::iterator it = fPackagesToBeActivated.begin(); + it != fPackagesToBeActivated.end(); ++it) { + (*it)->SetActive(true); + } + + for (PackageSet::iterator it = fPackagesToBeDeactivated.begin(); + it != fPackagesToBeDeactivated.end(); ++it) { + Package* package = *it; + _RemovePackage(package); + delete package; + } + + fPackagesToBeActivated.clear(); + fPackagesToBeDeactivated.clear(); +} + + void Volume::_HandleEntryCreatedOrRemoved(const BMessage* message, bool created) { @@ -371,13 +466,13 @@ INFORM("Volume::_PackagesEntryCreated(\"%s\")\n", name); entry.directory = fPackagesDirectoryRef.node; status_t error = entry.set_name(name); if (error != B_OK) { - ERROR("out of memory"); + ERROR("out of memory\n"); return; } Package* package = new(std::nothrow) Package; if (package == NULL) { - ERROR("out of memory"); + ERROR("out of memory\n"); return; } ObjectDeleter packageDeleter(package); @@ -392,46 +487,12 @@ INFORM("Volume::_PackagesEntryCreated(\"%s\")\n", name); fPackagesByNodeRef.Insert(package); packageDeleter.Detach(); - // activate package -// TODO: Don't do that here! - size_t nameLength = strlen(package->FileName()); - size_t requestSize = sizeof(PackageFSActivationChangeRequest) + nameLength; - PackageFSActivationChangeRequest* request - = (PackageFSActivationChangeRequest*)malloc(requestSize); - if (request == NULL) { - ERROR("out of memory"); + try { + fPackagesToBeActivated.insert(package); + } catch (std::bad_alloc& exception) { + ERROR("out of memory\n"); return; } - MemoryDeleter requestDeleter(request); - - request->itemCount = 1; - PackageFSActivationChangeItem& item = request->items[0]; - item.type = PACKAGE_FS_ACTIVATE_PACKAGE; - - item.packageDeviceID = package->NodeRef().device; - item.packageNodeID = package->NodeRef().node; - - item.nameLength = nameLength; - item.parentDeviceID = fPackagesDirectoryRef.device; - item.parentDirectoryID = fPackagesDirectoryRef.node; - strcpy(item.name, package->FileName()); - - int fd = OpenRootDirectory(); - if (fd < 0) { - ERROR("Volume::_PackagesEntryCreated(): failed to open root directory: " - "%s", strerror(fd)); - return; - } - FileDescriptorCloser fdCloser(fd); - - if (ioctl(fd, PACKAGE_FS_OPERATION_CHANGE_ACTIVATION, request, requestSize) - != 0) { - ERROR("Volume::_PackagesEntryCreated(): activate packages: %s\n", - strerror(errno)); - return; - } - - package->SetActive(true); } @@ -443,51 +504,50 @@ INFORM("Volume::_PackagesEntryRemoved(\"%s\")\n", name); if (package == NULL) return; - if (package->IsActive()) { - // deactivate the package -// TODO: Don't do that here! - size_t nameLength = strlen(package->FileName()); - size_t requestSize = sizeof(PackageFSActivationChangeRequest) - + nameLength; - PackageFSActivationChangeRequest* request - = (PackageFSActivationChangeRequest*)malloc(requestSize); - if (request == NULL) { - ERROR("out of memory"); - return; - } - MemoryDeleter requestDeleter(request); + // Remove the package from the packages-to-be-activated set, if it is in + // there (unlikely, unless we see a create-remove-create sequence). + PackageSet::iterator it = fPackagesToBeActivated.find(package); + if (it != fPackagesToBeActivated.end()) + fPackagesToBeActivated.erase(it); - request->itemCount = 1; - PackageFSActivationChangeItem& item = request->items[0]; - item.type = PACKAGE_FS_DEACTIVATE_PACKAGE; - - item.packageDeviceID = package->NodeRef().device; - item.packageNodeID = package->NodeRef().node; - - item.nameLength = nameLength; - item.parentDeviceID = fPackagesDirectoryRef.device; - item.parentDirectoryID = fPackagesDirectoryRef.node; - strcpy(item.name, package->FileName()); - - int fd = OpenRootDirectory(); - if (fd < 0) { - ERROR("Volume::_PackagesEntryRemoved(): failed to open root " - "directory: %s", strerror(fd)); - return; - } - FileDescriptorCloser fdCloser(fd); - - if (ioctl(fd, PACKAGE_FS_OPERATION_CHANGE_ACTIVATION, request, - requestSize) != 0) { - ERROR("Volume::_PackagesEntryRemoved(): activate packages: %s\n", - strerror(errno)); - return; - } + // If the package isn't active, just remove it for good. + if (!package->IsActive()) { + _RemovePackage(package); + delete package; + return; } + // The package must be deactivated. + try { + fPackagesToBeDeactivated.insert(package); + } catch (std::bad_alloc& exception) { + ERROR("out of memory\n"); + return; + } +} + + +void +Volume::_FillInActivationChangeItem(PackageFSActivationChangeItem* item, + PackageFSActivationChangeType type, Package* package, char*& nameBuffer) +{ + item->type = type; + item->packageDeviceID = package->NodeRef().device; + item->packageNodeID = package->NodeRef().node; + item->nameLength = package->FileName().Length(); + item->parentDeviceID = fPackagesDirectoryRef.device; + item->parentDirectoryID = fPackagesDirectoryRef.node; + item->name = nameBuffer; + strcpy(nameBuffer, package->FileName()); + nameBuffer += package->FileName().Length() + 1; +} + + +void +Volume::_RemovePackage(Package* package) +{ fPackagesByFileName.Remove(package); fPackagesByNodeRef.Remove(package); - delete package; } diff --git a/src/servers/package/Volume.h b/src/servers/package/Volume.h index 24eab04c6d..b83bc401cd 100644 --- a/src/servers/package/Volume.h +++ b/src/servers/package/Volume.h @@ -9,6 +9,8 @@ #define VOLUME_H +#include + #include #include #include @@ -68,10 +70,15 @@ public: void ProcessPendingNodeMonitorEvents(); + bool HasPendingPackageActivationChanges() const; + void ProcessPendingPackageActivationChanges(); + private: struct NodeMonitorEvent; typedef DoublyLinkedList NodeMonitorEventList; + typedef std::set PackageSet; + private: void _HandleEntryCreatedOrRemoved( const BMessage* message, bool created); @@ -82,6 +89,12 @@ private: void _PackagesEntryCreated(const char* name); void _PackagesEntryRemoved(const char* name); + void _FillInActivationChangeItem( + PackageFSActivationChangeItem* item, + PackageFSActivationChangeType type, + Package* package, char*& nameBuffer); + void _RemovePackage(Package* package); + status_t _ReadPackagesDirectory(); status_t _GetActivePackages(int fd); @@ -96,6 +109,8 @@ private: PackageNodeRefHashTable fPackagesByNodeRef; BLocker fPendingNodeMonitorEventsLock; NodeMonitorEventList fPendingNodeMonitorEvents; + PackageSet fPackagesToBeActivated; + PackageSet fPackagesToBeDeactivated; };