* Improved BNotification API.

- No more manual memory management.
   - Make it clear who keeps or releases ownership of arguments passed.
   - Copy icon, arguments and entry_refs.
   - Do not expose implementation details (What do the BLists contain?!).
   - BRoster takes const BNotification& and bigtime_t timeout.

 * BRoster::Notify():
   - Proper error handling.
   - Fixed documentation.

 * Adjusted notify:
   - Renamed fOk to fHasGoodArguments.
   - The "const char*" members were really "char*" members (self-managed).
   - free() is NULL-safe.
   - fRefs contains BEntries, so passing void* to delete does no good.
   - Adjustments to the changed API.
   - Coding style fixes.

 * notification_server:
   - Adjustment to the new type for timeout.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@36952 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/os/app/Notification.h

@@ -7,10 +7,8 @@
 
 
 #include <Entry.h>
-
+#include <List.h>
-
+#include <String.h>
-class BBitmap;
-class BList;
 
 
 // notification types
@@ -21,6 +19,8 @@ enum notification_type {
 	B_PROGRESS_NOTIFICATION
 };
 
+class BBitmap;
+
 
 class BNotification {
 public:
@@ -47,29 +47,33 @@ public:
 			const char*			OnClickApp() const;
 			void				SetOnClickApp(const char* app);
 
-			entry_ref*			OnClickFile() const;
+			const entry_ref*	OnClickFile() const;
-			void				SetOnClickFile(const entry_ref* file);
+			status_t			SetOnClickFile(const entry_ref* file);
 
-			BList*				OnClickRefs() const;
+			status_t			AddOnClickRef(const entry_ref* ref);
-			void				AddOnClickRef(const entry_ref* ref);
+			status_t			AddOnClickRef(const entry_ref& ref);
+			int32				CountOnClickRefs() const;
+			const entry_ref*	OnClickRefAt(int32 index) const;
 
-			BList*				OnClickArgv() const;
+			status_t			AddOnClickArg(const char* arg);
-			void				AddOnClickArg(const char* arg);
+			int32				CountOnClickArgs() const;
+			const char*			OnClickArgAt(int32 index) const;
 
-			BBitmap*			Icon() const;
+			const BBitmap*		Icon() const;
-			void				SetIcon(BBitmap* icon);
+			status_t			SetIcon(const BBitmap* icon);
 
 private:
 			notification_type	fType;
-			char*				fAppName;
+			BString				fAppName;
-			char*				fTitle;
+			BString				fTitle;
-			char*				fContent;
+			BString				fContent;
-			char*				fID;
+			BString				fID;
 			float				fProgress;
-			char*				fApp;
+
+			BString				fApp;
 			entry_ref*			fFile;
-			BList*				fRefs;
+			BList				fRefs;
-			BList*				fArgv;
+			BList				fArgv;
 			BBitmap*			fBitmap;
 };
 

headers/os/app/Roster.h

@@ -118,8 +118,8 @@ class BRoster {
 					const char *appSig = 0) const;
 
 		// notifications
-		status_t Notify(BNotification* notification,
+		status_t Notify(const BNotification& notification,
-					int32 timeout = -1) const;
+					bigtime_t timeout = -1) const;
 
 		// private/reserved stuff starts here
 		class Private;

src/bin/notify.cpp

@@ -4,7 +4,8 @@
  * Distributed under the terms of the MIT License.
  *
  * Authors:
- *              Pier Luigi Fiorini <[email protected]>
+ *		Pier Luigi Fiorini <[email protected]>
+ *		Stephan Aßmus <[email protected]>
  */
 
 #include <stdio.h>
@@ -45,20 +46,21 @@ public:
 	virtual void				ReadyToRun();
 	virtual void				ArgvReceived(int32 argc, char** argv);
 
-			bool				GoodArguments() const { return fOk; }
+			bool				HasGoodArguments() const
+									{ return fHasGoodArguments; }
 
 private:
-			bool				fOk;
+			bool				fHasGoodArguments;
 			notification_type	fType;
-			const char*			fAppName;
+			char*				fAppName;
-			const char*			fTitle;
+			char*				fTitle;
-			const char*			fMsgId;
+			char*				fMsgId;
 			float				fProgress;
-			int32				fTimeout;
+			bigtime_t			fTimeout;
-			const char*			fIconFile;
+			char*				fIconFile;
 			entry_ref			fFileRef;
-			const char*			fMessage;
+			char*				fMessage;
-			const char*			fApp;
+			char*				fApp;
 			bool				fHasFile;
 			entry_ref			fFile;
 			BList*				fRefs;
@@ -72,7 +74,7 @@ private:
 NotifyApp::NotifyApp()
 	:
 	BApplication(kSignature),
-	fOk(false),
+	fHasGoodArguments(false),
 	fType(B_INFORMATION_NOTIFICATION),
 	fAppName(NULL),
 	fTitle(NULL),
@@ -91,30 +93,19 @@ NotifyApp::NotifyApp()
 
 NotifyApp::~NotifyApp()
 {
-	if (fAppName)
+	free(fAppName);
-		free((void*)fAppName);
+	free(fTitle);
-	if (fTitle)
+	free(fMsgId);
-		free((void*)fTitle);
+	free(fIconFile);
-	if (fMsgId)
+	free(fMessage);
-		free((void*)fMsgId);
+	free(fApp);
-	if (fIconFile)
-		free((void*)fIconFile);
-	if (fMessage)
-		free((void*)fMessage);
-	if (fApp)
-		free((void*)fApp);
 
-	int32 i;
+	for (int32 i = 0; void* item = fRefs->ItemAt(i); i++)
-	void* item;
+		delete (BEntry*)item;
-
-	for (i = 0; item = fRefs->ItemAt(i); i++)
-		delete item;
 	delete fRefs;
 
-	for (i = 0; item = fArgv->ItemAt(i); i++) {
+	for (int32 i = 0; void* item = fArgv->ItemAt(i); i++)
-		if (item != NULL)
+		free(item);
-			free(item);
-	}
 	delete fArgv;
 }
 
@@ -143,17 +134,17 @@ NotifyApp::ArgvReceived(int32 argc, char** argv)
 					if (strncmp(kTypeNames[i], argument, strlen(argument)) == 0)
 						fType = (notification_type)i;
 				}
-			} else if (strcmp(option, "app") == 0) {
+			} else if (strcmp(option, "app") == 0)
 				fAppName = strdup(argument);
-			} else if (strcmp(option, "title") == 0) {
+			else if (strcmp(option, "title") == 0)
 				fTitle = strdup(argument);
-			} else if (strcmp(option, "messageID") == 0) {
+			else if (strcmp(option, "messageID") == 0)
 				fMsgId = strdup(argument);
-			} else if (strcmp(option, "progress") == 0) {
+			else if (strcmp(option, "progress") == 0)
 				fProgress = atof(argument);
-			} else if (strcmp(option, "timeout") == 0) {
+			else if (strcmp(option, "timeout") == 0)
-				fTimeout = atol(argument);
+				fTimeout = atol(argument) * 1000000;
-			} else if (strcmp(option, "icon") == 0) {
+			else if (strcmp(option, "icon") == 0) {
 				fIconFile = strdup(argument);
 
 				if (get_ref_for_path(fIconFile, &fFileRef) < B_OK) {
@@ -177,17 +168,18 @@ NotifyApp::ArgvReceived(int32 argc, char** argv)
 					return;
 				}
 
-				fRefs->AddItem((void*)new BEntry(&ref));
+				fRefs->AddItem(new BEntry(&ref));
 			} else if (strcmp(option, "onClickArgv") == 0)
-				fArgv->AddItem((void*)strdup(argument));
+				fArgv->AddItem(strdup(argument));
 			else {
 				// Unrecognized option
 				fprintf(stderr, "Unrecognized option --%s\n\n", option);
 				return;
 			}
-		} else
+		} else {
 			// Option doesn't start with '--'
 			break;
+		}
 
 		if (index == kArgCount) {
 			// No text argument provided, only '--' arguments
@@ -197,17 +189,17 @@ NotifyApp::ArgvReceived(int32 argc, char** argv)
 	}
 
 	// Check for missing arguments
-	if (!fAppName) {
+	if (fAppName == NULL) {
 		fprintf(stderr, "Missing --app argument!\n\n");
 		return;
 	}
-	if (!fTitle) {
+	if (fTitle == NULL) {
 		fprintf(stderr, "Missing --title argument!\n\n");
 		return;
 	}
 
 	fMessage = strdup(argv[index]);
-	fOk = true;
+	fHasGoodArguments = true;
 }
 
 void
@@ -262,47 +254,50 @@ NotifyApp::_GetBitmap(const entry_ref* ref) const
 void
 NotifyApp::ReadyToRun()
 {
-	if (GoodArguments()) {
+	if (HasGoodArguments()) {
-		BNotification* msg = new BNotification(fType);
+		BNotification notification(fType);
-		msg->SetApplication(fAppName);
+		notification.SetApplication(fAppName);
-		msg->SetTitle(fTitle);
+		notification.SetTitle(fTitle);
-		msg->SetContent(fMessage);
+		notification.SetContent(fMessage);
 
-		if (fMsgId)
+		if (fMsgId != NULL)
-			msg->SetMessageID(fMsgId);
+			notification.SetMessageID(fMsgId);
 
 		if (fType == B_PROGRESS_NOTIFICATION)
-			msg->SetProgress(fProgress);
+			notification.SetProgress(fProgress);
 
-		if (fIconFile) {
+		if (fIconFile != NULL) {
 			BBitmap* bitmap = _GetBitmap(&fFileRef);
-			if (bitmap)
+			if (bitmap) {
-				msg->SetIcon(bitmap);
+				notification.SetIcon(bitmap);
+				delete bitmap;
+			}
 		}
 
-		if (fApp)
+		if (fApp != NULL)
-			msg->SetOnClickApp(fApp);
+			notification.SetOnClickApp(fApp);
 
 		if (fHasFile)
-			msg->SetOnClickFile(&fFile);
+			notification.SetOnClickFile(&fFile);
 
-		int32 i;
+		for (int32 i = 0; void* item = fRefs->ItemAt(i); i++) {
-		void* item;
-
-		for (i = 0; item = fRefs->ItemAt(i); i++) {
 			BEntry* entry = (BEntry*)item;
 
 			entry_ref ref;
 			if (entry->GetRef(&ref) == B_OK)
-				msg->AddOnClickRef(&ref);
+				notification.AddOnClickRef(&ref);
 		}
 
-		for (i = 0; item = fArgv->ItemAt(i); i++) {
+		for (int32 i = 0; void* item = fArgv->ItemAt(i); i++) {
 			const char* arg = (const char*)item;
-			msg->AddOnClickArg(arg);
+			notification.AddOnClickArg(arg);
 		}
 
-		be_roster->Notify(msg, fTimeout);
+		status_t ret = be_roster->Notify(notification, fTimeout);
+		if (ret != B_OK) {
+			fprintf(stderr, "Failed to deliver notification: %s\n",
+				strerror(ret));
+		}
 	} else
 		_Usage();
 
@@ -318,7 +313,7 @@ main(int argc, char** argv)
 		return kErrorInitFail;
 
 	app.Run();
-	if (!app.GoodArguments())
+	if (!app.HasGoodArguments())
 		return kErrorArgumentsFail;
 
 	return 0;

src/kits/app/Notification.cpp

@@ -4,48 +4,40 @@
  *
  * Authors:
  *		Pier Luigi Fiorini, [email protected]
+ *		Stephan Aßmus <[email protected]>
  */
 
+
+#include <Notification.h>
+
+#include <new>
+
 #include <stdlib.h>
 #include <string.h>
 
 #include <Bitmap.h>
-#include <List.h>
 #include <Message.h>
-#include <Notification.h>
 
 
 BNotification::BNotification(notification_type type)
 	:
 	fType(type),
-	fAppName(NULL),
-	fTitle(NULL),
-	fContent(NULL),
-	fID(NULL),
-	fApp(NULL),
 	fFile(NULL),
 	fBitmap(NULL)
 {
-	fRefs = new BList();
-	fArgv = new BList();
 }
 
 
 BNotification::~BNotification()
 {
-	if (fAppName)
+	delete fFile;
-		free(fAppName);
+	delete fBitmap;
-	if (fTitle)
-		free(fTitle);
-	if (fContent)
-		free(fContent);
-	if (fID)
-		free(fID);
-	if (fApp)
-		free(fApp);
 
-	delete fRefs;
+	for (int32 i = fRefs.CountItems() - 1; i >= 0; i--)
-	delete fArgv;
+		delete (entry_ref*)fRefs.ItemAtFast(i);
+
+	for (int32 i = fArgv.CountItems() - 1; i >= 0; i--)
+		free(fArgv.ItemAtFast(i));
 }
 
 
@@ -66,11 +58,7 @@ BNotification::Application() const
 void
 BNotification::SetApplication(const char* app)
 {
-	free(fAppName);
+	fAppName = app;
-	fAppName = NULL;
-
-	if (app)
-		fAppName = strdup(app);
 }
 
 
@@ -84,11 +72,7 @@ BNotification::Title() const
 void
 BNotification::SetTitle(const char* title)
 {
-	free(fTitle);
+	fTitle = title;
-	fTitle = NULL;
-
-	if (title)
-		fTitle = strdup(title);
 }
 
 
@@ -102,11 +86,7 @@ BNotification::Content() const
 void
 BNotification::SetContent(const char* content)
 {
-	free(fContent);
+	fContent = content;
-	fContent = NULL;
-
-	if (content)
-		fContent = strdup(content);
 }
 
 
@@ -120,11 +100,7 @@ BNotification::MessageID() const
 void
 BNotification::SetMessageID(const char* id)
 {
-	free(fID);
+	fID = id;
-	fID = NULL;
-
-	if (id)
-		fID = strdup(id);
 }
 
 
@@ -152,65 +128,115 @@ BNotification::OnClickApp() const
 void
 BNotification::SetOnClickApp(const char* app)
 {
-	free(fApp);
+	fApp = app;
-	fApp = NULL;
-
-	if (app)
-		fApp = strdup(app);
 }
 
 
-entry_ref*
+const entry_ref*
 BNotification::OnClickFile() const
 {
 	return fFile;
 }
 
 
-void
+status_t
 BNotification::SetOnClickFile(const entry_ref* file)
 {
-	fFile = (entry_ref*)file;
+	delete fFile;
+
+	if (file != NULL) {
+		fFile = new(std::nothrow) entry_ref(*file);
+		if (fFile == NULL)
+			return B_NO_MEMORY;
+	} else
+		fFile = NULL;
+
+	return B_OK;
 }
 
 
-BList*
+status_t
-BNotification::OnClickRefs() const
-{
-	return fRefs;
-}
-
-
-void
 BNotification::AddOnClickRef(const entry_ref* ref)
 {
-	fRefs->AddItem((void*)ref);
+	if (ref == NULL)
+		return B_BAD_VALUE;
+
+	return AddOnClickRef(*ref);
 }
 
 
-BList*
+int32
-BNotification::OnClickArgv() const
+BNotification::CountOnClickRefs() const
 {
-	return fArgv;
+	return fRefs.CountItems();
 }
 
 
-void
+const entry_ref*
+BNotification::OnClickRefAt(int32 index) const
+{
+	return (entry_ref*)fArgv.ItemAt(index);
+}
+
+
+status_t
+BNotification::AddOnClickRef(const entry_ref& ref)
+{
+	entry_ref* clonedRef = new(std::nothrow) entry_ref(ref);
+	if (clonedRef == NULL || !fRefs.AddItem(clonedRef))
+		return B_NO_MEMORY;
+
+	return B_OK;
+}
+
+
+status_t
 BNotification::AddOnClickArg(const char* arg)
 {
-	fArgv->AddItem((void*)arg);
+	if (arg == NULL)
+		return B_BAD_VALUE;
+
+	char* clonedArg = strdup(arg);
+	if (clonedArg == NULL || !fArgv.AddItem(clonedArg))
+		return B_NO_MEMORY;
+
+	return B_OK;
 }
 
 
-BBitmap*
+int32
+BNotification::CountOnClickArgs() const
+{
+	return fArgv.CountItems();
+}
+
+
+const char*
+BNotification::OnClickArgAt(int32 index) const
+{
+	return (char*)fArgv.ItemAt(index);
+}
+
+
+const BBitmap*
 BNotification::Icon() const
 {
 	return fBitmap;
 }
 
 
-void
+status_t
-BNotification::SetIcon(BBitmap* icon)
+BNotification::SetIcon(const BBitmap* icon)
 {
-	fBitmap = icon;
+	delete fBitmap;
+
+	if (icon != NULL) {
+		fBitmap = new(std::nothrow) BBitmap(icon);
+		if (fBitmap == NULL)
+			return B_NO_MEMORY;
+		return fBitmap->InitCheck();
+	}
+
+	fBitmap = NULL;
+	return B_OK;
 }

src/kits/app/Roster.cpp

@@ -1655,8 +1655,8 @@ BRoster::AddToRecentFolders(const entry_ref* folder, const char* appSig) const
 
 /*! \brief Sends a notification to the notification_server.
 
-	The notification is delivered synchronously to the notification_server,
+	The notification is delivered asynchronously to the notification_server,
-	that will displays it according to its settings and filters.
+	which will displays it according to its settings and filters.
 
 	\param notification Notification message.
 	\param timeout Seconds after the message fades out.
@@ -1664,50 +1664,69 @@ BRoster::AddToRecentFolders(const entry_ref* folder, const char* appSig) const
 	- \c B_OK: Everything went fine.
 	- \c B_BAD_PORT_ID: A connection to notification_server could not be
 	  established or the server is not up and running anymore.
+	- \c Other errors: Building the message from the notification failed.
 */
 status_t
-BRoster::Notify(BNotification* notification, int32 timeout) const
+BRoster::Notify(const BNotification& notification, bigtime_t timeout) const
 {
+	// TODO: Add BArchivable support to BNotification and use it here.
 	BMessage msg(kNotificationMessage);
-	msg.AddInt32("type", (int32)notification->Type());
+	status_t ret = msg.AddInt32("type", (int32)notification.Type());
-	msg.AddString("app", notification->Application());
+	if (ret == B_OK)
-	msg.AddString("title", notification->Title());
+		ret = msg.AddString("app", notification.Application());
-	msg.AddString("content", notification->Content());
+	if (ret == B_OK)
+		ret = msg.AddString("title", notification.Title());
+	if (ret == B_OK)
+		ret = msg.AddString("content", notification.Content());
 
-	if (notification->MessageID())
+	if (ret == B_OK && notification.MessageID() != NULL)
-		msg.AddString("messageID", notification->MessageID());
+		ret = msg.AddString("messageID", notification.MessageID());
 
-	if (notification->Type() == B_PROGRESS_NOTIFICATION)
+	if (ret == B_OK && notification.Type() == B_PROGRESS_NOTIFICATION)
-		msg.AddFloat("progress", notification->Progress());
+		ret = msg.AddFloat("progress", notification.Progress());
 
-	if (notification->OnClickApp())
+	if (ret == B_OK && notification.OnClickApp() != NULL)
-		msg.AddString("onClickApp", notification->OnClickApp());
+		ret = msg.AddString("onClickApp", notification.OnClickApp());
-	if (notification->OnClickFile())
+	if (ret == B_OK && notification.OnClickFile() != NULL)
-		msg.AddRef("onClickFile", notification->OnClickFile());
+		ret = msg.AddRef("onClickFile", notification.OnClickFile());
 
-	int32 i;
+	if (ret == B_OK) {
+		for (int32 i = 0; i < notification.CountOnClickRefs(); i++) {
+			ret = msg.AddRef("onClickRef", notification.OnClickRefAt(i));
+			if (ret != B_OK)
+				break;
+		}
+	}
 
-	BList* refs = notification->OnClickRefs();
+	if (ret == B_OK) {
-	for (i = 0; i < refs->CountItems(); i++)
+		for (int32 i = 0; i < notification.CountOnClickArgs(); i++) {
-		msg.AddRef("onClickRef", (entry_ref*)refs->ItemAt(i));
+			ret = msg.AddString("onClickArgv", notification.OnClickArgAt(i));
+			if (ret != B_OK)
+				break;
+		}
+	}
 
-	BList* argv = notification->OnClickArgv();
+	if (ret == B_OK) {
-	for (i = 0; i < argv->CountItems(); i++)
+		const BBitmap* icon = notification.Icon();
-		msg.AddString("onClickArgv", (const char*)argv->ItemAt(i));
+		if (icon != NULL) {
-
+			BMessage archive;
-	BBitmap* icon = notification->Icon();
+			ret = icon->Archive(&archive);
-
+			if (ret == B_OK)
-	BMessage archive;
+				ret = msg.AddMessage("icon", &archive);
-	if (icon && icon->Archive(&archive) == B_OK)
+		}
-		msg.AddMessage("icon", &archive);
+	}
 
 	// Custom time out
-	if (timeout > 0)
+	if (ret == B_OK && timeout > 0)
-		msg.AddInt32("timeout", timeout);
+		ret = msg.AddInt64("timeout", timeout);
 
 	// Send message
-	BMessenger server(kNotificationServerSignature);
+	if (ret == B_OK) {
-	return server.SendMessage(&msg);
+		BMessenger server(kNotificationServerSignature);
+		ret = server.SendMessage(&msg);
+	}
+
+	return ret;
 }
 
 

src/servers/notification/NotificationView.cpp

@@ -9,6 +9,7 @@
  *		Michael Davidson, slaad@bong.com.au
  *		Mikael Eiman, mikael@eiman.tv
  *		Pier Luigi Fiorini, pierluigi.fiorini@gmail.com
+ *		Stephan Aßmus <superstippi@gmx.de>
  */
 
 #include <stdlib.h>
@@ -122,14 +123,13 @@ NotificationView::AttachedToWindow()
 {
 	BMessage msg(kRemoveView);
 	msg.AddPointer("view", this);
-	int32 timeout = -1;
+	bigtime_t timeout = -1;
 
-	if (fDetails->FindInt32("timeout", &timeout) != B_OK)
+	if (fDetails->FindInt64("timeout", &timeout) != B_OK)
-		timeout = fParent->Timeout();
+		timeout = fParent->Timeout() * 1000000;
-	bigtime_t delay = timeout * 1000 * 1000;
+
-	
+	if (timeout > 0)
-	if (delay > 0)
+		fRunner = new BMessageRunner(BMessenger(Parent()), &msg, timeout, 1);
-		fRunner = new BMessageRunner(BMessenger(Parent()), &msg, delay, 1);
 }