PackageInstaller: Another round of cleanup.

src/apps/packageinstaller/PackageInstall.cpp

@@ -24,20 +24,21 @@
 
 
 static int32
-install_function(void *data)
+install_function(void* data)
 {
 	// TODO: Inform if already one thread is running
-	PackageInstall *install = static_cast<PackageInstall *>(data);
 	if (data == NULL)
 		return -1;
 
+	PackageInstall* install = static_cast<PackageInstall*>(data);
 	install->Install();
 	return 0;
 }
 
 
-PackageInstall::PackageInstall(PackageView *parent)
+PackageInstall::PackageInstall(PackageView* parent)
-	: fParent(parent),
+	:
+	fParent(parent),
 	fThreadId(-1),
 	fCurrentScript(NULL)
 {
@@ -58,8 +59,8 @@ PackageInstall::Start()
 	if (fThreadId > -1) {
 		ret = B_BUSY;
 	} else {
-		fThreadId = spawn_thread(install_function, "install_package", B_NORMAL_PRIORITY,
+		fThreadId = spawn_thread(install_function, "install_package",
-				static_cast<void *>(this));
+			B_NORMAL_PRIORITY, this);
 		resume_thread(fThreadId);
 	}
 	fIdLocker.Unlock();
@@ -71,6 +72,9 @@ PackageInstall::Start()
 void
 PackageInstall::Stop()
 {
+	// TODO: Argh! No killing of threads!! That leaks resources which they
+	// allocated. Rather inform them they need to quit, which they do at the
+	// next convenient time, then use wait_for_thread() here.
 	fIdLocker.Lock();
 	if (fThreadId > -1) {
 		kill_thread(fThreadId);
@@ -95,21 +99,40 @@ void
 PackageInstall::Install()
 {
 	// A message sending wrapper around _Install()
-	uint32 msg = _Install();
+	uint32 code = _Install();
-	if (fParent && fParent->Looper())
+
-		fParent->Looper()->PostMessage(new BMessage(msg), fParent);
+	BMessenger messenger(fParent);
+	if (messenger.IsValid()) {
+		BMessage message(code);
+		messenger.SendMessage(&message);
+	}
+}
+
+
+static inline BString
+get_item_progress_string(uint32 index, uint32 total) 
+{
+	BString label(B_TRANSLATE("%index% of %total%"));
+	BString indexString;
+	indexString << (index + 1);
+	BString totalString;
+	totalString << total;
+	label.ReplaceAll("%index%", indexString);
+	label.ReplaceAll("%total%", totalString);
+	return label;
 }
 
 
 uint32
 PackageInstall::_Install()
 {
-	PackageInfo *info = fParent->GetPackageInfo();
+	PackageInfo* info = fParent->GetPackageInfo();
-	pkg_profile *type = static_cast<pkg_profile *>(info->GetProfile(
+	pkg_profile* type = static_cast<pkg_profile*>(info->GetProfile(
-		fParent->GetCurrentType()));
+		fParent->CurrentType()));
-	uint32 n = type->items.CountItems(), m = info->GetScriptCount();
+	uint32 n = type->items.CountItems();
+	uint32 m = info->GetScriptCount();
 
-	PackageStatus *progress = fParent->GetStatusWindow();
+	PackageStatus* progress = fParent->StatusWindow();
 	progress->Reset(n + m + 5);
 
 	progress->StageStep(1, B_TRANSLATE("Preparing package"));
@@ -119,7 +142,7 @@ PackageInstall::_Install()
 	status_t err = packageInfo.InitCheck();
 	if (err == B_OK) {
 		// The package is already installed, inform the user
-		BAlert *reinstall = new BAlert("reinstall",
+		BAlert* reinstall = new BAlert("reinstall",
 			B_TRANSLATE("The given package seems to be already installed on "
 				"your system. Would you like to uninstall the existing one "
 				"and continue the installation?"),
@@ -131,13 +154,15 @@ PackageInstall::_Install()
 			// Uninstall the package
 			err = packageInfo.Uninstall();
 			if (err != B_OK) {
-				fprintf(stderr, "Error on uninstall\n");
+				fprintf(stderr, "Error uninstalling previously installed "
+					"package: %s\n", strerror(err));
 				return P_MSG_I_ERROR;
 			}
 
 			err = packageInfo.SetTo(info->GetName(), info->GetVersion(), true);
 			if (err != B_OK) {
-				fprintf(stderr, "Error on SetTo\n");
+				fprintf(stderr, "Error marking installation of package: "
+					"%s\n", strerror(err));
 				return P_MSG_I_ERROR;
 			}
 		} else {
@@ -147,7 +172,8 @@ PackageInstall::_Install()
 	} else if (err == B_ENTRY_NOT_FOUND) {
 		err = packageInfo.SetTo(info->GetName(), info->GetVersion(), true);
 		if (err != B_OK) {
-			fprintf(stderr, "Error on SetTo\n");
+				fprintf(stderr, "Error marking installation of package: "
+					"%s\n", strerror(err));
 			return P_MSG_I_ERROR;
 		}
 	} else if (progress->Stopped()) {
@@ -160,11 +186,6 @@ PackageInstall::_Install()
 	progress->StageStep(1, B_TRANSLATE("Installing files and folders"));
 
 	// Install files and directories
-	PackageItem *iter;
-	ItemState state;
-	uint32 i;
-	int32 choice;
-	BString label;
 
 	packageInfo.SetName(info->GetName());
 	// TODO: Here's a small problem, since right now it's not quite sure
@@ -181,20 +202,20 @@ PackageInstall::_Install()
 
 	fItemExistsPolicy = P_EXISTS_NONE;
 
-	const char *installPath = fParent->GetCurrentPath()->Path();
+	const char *installPath = fParent->CurrentPath()->Path();
-	for (i = 0; i < n; i++) {
+	for (uint32 i = 0; i < n; i++) {
-		state.Reset(fItemExistsPolicy); // Reset the current item state
+		ItemState state(fItemExistsPolicy);
-		iter = static_cast<PackageItem *>(type->items.ItemAt(i));
+		PackageItem* item = static_cast<PackageItem*>(type->items.ItemAt(i));
 
-		err = iter->DoInstall(installPath, &state);
+		err = item->DoInstall(installPath, &state);
 		if (err == B_FILE_EXISTS) {
 			// Writing to path failed because path already exists - ask the user
 			// what to do and retry the writing process
-			choice = fParent->ItemExists(*iter, state.destination,
+			int32 choice = fParent->ItemExists(*item, state.destination,
 				fItemExistsPolicy);
 			if (choice != P_EXISTS_ABORT) {
 				state.policy = choice;
-				err = iter->DoInstall(installPath, &state);
+				err = item->DoInstall(installPath, &state);
 			}
 		}
 
@@ -205,42 +226,44 @@ PackageInstall::_Install()
 
 		if (progress->Stopped())
 			return P_MSG_I_ABORT;
-		label = "";
-		label << (uint32)(i + 1) << " of " << (uint32)n;
-		progress->StageStep(1, NULL, label.String());
 
+		// Update progress
+		progress->StageStep(1, NULL, get_item_progress_string(i, n).String());
+
+		// Mark installed item in packageInfo
 		packageInfo.AddItem(state.destination.Path());
 	}
 
 	progress->StageStep(1, B_TRANSLATE("Running post-installation scripts"),
 		 "");
 
-	PackageScript *scr;
-	status_t status;
 	// Run all scripts
-	for (i = 0; i < m; i++) {
+	// TODO: Change current working directory to installation location!
-		scr = info->GetScript(i);
+	for (uint32 i = 0; i < m; i++) {
+		PackageScript* script = info->GetScript(i);
 
 		fCurrentScriptLocker.Lock();
-		fCurrentScript = scr;
+		fCurrentScript = script;
 
-		if (scr->DoInstall() != B_OK) {
+		status_t status = script->DoInstall();
-			fprintf(stderr, "Error while running script\n");
+		if (status != B_OK) {
+			fprintf(stderr, "Error while running script: %s\n",
+				strerror(status));
 			return P_MSG_I_ERROR;
 		}
 		fCurrentScriptLocker.Unlock();
 
-		wait_for_thread(scr->GetThreadId(), &status);
+		wait_for_thread(script->GetThreadId(), &status);
+
 		fCurrentScriptLocker.Lock();
-		scr->SetThreadId(-1);
+		script->SetThreadId(-1);
 		fCurrentScript = NULL;
 		fCurrentScriptLocker.Unlock();
 
 		if (progress->Stopped())
 			return P_MSG_I_ABORT;
-		label = "";
+
-		label << (uint32)(i + 1) << " of " << (uint32)m;
+		progress->StageStep(1, NULL, get_item_progress_string(i, m).String());
-		progress->StageStep(1, NULL, label.String());
 	}
 
 	progress->StageStep(1, B_TRANSLATE("Finishing installation"), "");

src/apps/packageinstaller/PackageItem.h

@@ -53,15 +53,15 @@ extern status_t inflate_data(uint8* in, uint32 inSize, uint8* out,
 
 
 struct ItemState {
-	ItemState() : policy(P_EXISTS_NONE), status(B_NO_INIT) {}
+	ItemState(uint8 _policy)
-	~ItemState() {}
+		:
-
+		policy(_policy),
-	inline void Reset(int32 currentPolicy)
+		status(B_NO_INIT)
+	{
+	}
+
+	~ItemState()
 	{
-		destination.Unset();
-		parent.Unset();
-		status = B_NO_INIT;
-		policy = currentPolicy;
 	}
 
 	BPath		destination;

src/apps/packageinstaller/PackageView.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2007-2010, Haiku, Inc.
+ * Copyright 2007-2014, Haiku, Inc.
  * Distributed under the terms of the MIT license.
  *
  * Author:
@@ -13,11 +13,14 @@
 #include "PackageView.h"
 
 #include <Alert.h>
+#include <Box.h>
 #include <Button.h>
 #include <Catalog.h>
 #include <Directory.h>
+#include <FilePanel.h>
 #include <FindDirectory.h>
 #include <Locale.h>
+#include <MenuField.h>
 #include <MenuItem.h>
 #include <Path.h>
 #include <PopUpMenu.h>

src/apps/packageinstaller/PackageView.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2007-2009, Haiku, Inc.
+ * Copyright 2007-2014, Haiku, Inc.
  * Distributed under the terms of the MIT license.
  *
  * Author:
@@ -13,12 +13,12 @@
 #include "PackageInstall.h"
 #include "PackageStatus.h"
 
-#include <Box.h>
-#include <Button.h>
-#include <FilePanel.h>
-#include <MenuField.h>
 #include <View.h>
 
+class BBox;
+class BButton;
+class BFilePanel;
+class BMenuField;
 class BPopUpMenu;
 class BTextView;
 
@@ -31,39 +31,45 @@ enum {
 };
 
 class PackageView : public BView {
-	public:
+public:
-		PackageView(BRect frame, const entry_ref *ref);
+								PackageView(BRect frame, const entry_ref* ref);
-		~PackageView();
+	virtual						~PackageView();
 
-		void AttachedToWindow();
+	virtual	void				AttachedToWindow();
-		void MessageReceived(BMessage *msg);
+	virtual	void				MessageReceived(BMessage* message);
 
-		int32 ItemExists(PackageItem &item, BPath &path, int32 &policy);
+			int32				ItemExists(PackageItem& item,
+									BPath& path, int32& policy);
 
-		BPath *GetCurrentPath()          { return &fCurrentPath; }
+			BPath*				CurrentPath()
-		PackageInfo *GetPackageInfo()    { return &fInfo; }
+									{ return &fCurrentPath; }
-		uint32 GetCurrentType()          { return fCurrentType; }
+			PackageInfo*		GetPackageInfo()
-		PackageStatus *GetStatusWindow() { return fStatusWindow; }
+									{ return &fInfo; }
+			uint32				CurrentType() const
+									{ return fCurrentType; }
+			PackageStatus*		StatusWindow()
+									{ return fStatusWindow; }
 
-	private:
+private:
-		void _InitView();
+			void				_InitView();
-		void _InitProfiles();
+			void				_InitProfiles();
 
-		status_t _GroupChanged(int32 index);
+			status_t			_GroupChanged(int32 index);
 
-		BPopUpMenu *fInstallTypes;
+private:
-		BTextView *fInstallDesc;
+			BPopUpMenu*			fInstallTypes;
-		BPopUpMenu *fDestination;
+			BTextView*			fInstallDesc;
-		BMenuField *fDestField;
+			BPopUpMenu*			fDestination;
-		BButton *fInstall;
+			BMenuField*			fDestField;
+			BButton*			fInstall;
 
-		BFilePanel *fOpenPanel;
+			BFilePanel*			fOpenPanel;
-		BPath fCurrentPath;
+			BPath				fCurrentPath;
-		uint32 fCurrentType;
+			uint32				fCurrentType;
 
-		PackageInfo fInfo;
+			PackageInfo			fInfo;
-		PackageStatus *fStatusWindow;
+			PackageStatus*		fStatusWindow;
-		PackageInstall fInstallProcess;
+			PackageInstall		fInstallProcess;
 };
 
 #endif	// PACKAGE_VIEW_H