From f33637d9a8d0a6c080d85d375c8143cd13277d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Thu, 27 May 2010 17:50:12 +0000 Subject: [PATCH] * 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 | 42 ++--- headers/os/app/Roster.h | 4 +- src/bin/notify.cpp | 131 +++++++------- src/kits/app/Notification.cpp | 160 ++++++++++-------- src/kits/app/Roster.cpp | 83 +++++---- src/servers/notification/NotificationView.cpp | 14 +- 6 files changed, 239 insertions(+), 195 deletions(-) diff --git a/headers/os/app/Notification.h b/headers/os/app/Notification.h index 6c014e66c4..05e925503d 100644 --- a/headers/os/app/Notification.h +++ b/headers/os/app/Notification.h @@ -7,10 +7,8 @@ #include - - -class BBitmap; -class BList; +#include +#include // 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; - void SetOnClickFile(const entry_ref* file); + const entry_ref* OnClickFile() const; + status_t SetOnClickFile(const entry_ref* file); - BList* OnClickRefs() const; - void AddOnClickRef(const entry_ref* ref); + status_t 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; - void AddOnClickArg(const char* arg); + status_t AddOnClickArg(const char* arg); + int32 CountOnClickArgs() const; + const char* OnClickArgAt(int32 index) const; - BBitmap* Icon() const; - void SetIcon(BBitmap* icon); + const BBitmap* Icon() const; + status_t SetIcon(const BBitmap* icon); private: notification_type fType; - char* fAppName; - char* fTitle; - char* fContent; - char* fID; + BString fAppName; + BString fTitle; + BString fContent; + BString fID; float fProgress; - char* fApp; + + BString fApp; entry_ref* fFile; - BList* fRefs; - BList* fArgv; + BList fRefs; + BList fArgv; BBitmap* fBitmap; }; diff --git a/headers/os/app/Roster.h b/headers/os/app/Roster.h index 641c9c64b6..e55e5189dc 100644 --- a/headers/os/app/Roster.h +++ b/headers/os/app/Roster.h @@ -118,8 +118,8 @@ class BRoster { const char *appSig = 0) const; // notifications - status_t Notify(BNotification* notification, - int32 timeout = -1) const; + status_t Notify(const BNotification& notification, + bigtime_t timeout = -1) const; // private/reserved stuff starts here class Private; diff --git a/src/bin/notify.cpp b/src/bin/notify.cpp index 1528f39ad3..7ad9170c10 100644 --- a/src/bin/notify.cpp +++ b/src/bin/notify.cpp @@ -4,7 +4,8 @@ * Distributed under the terms of the MIT License. * * Authors: - * Pier Luigi Fiorini + * Pier Luigi Fiorini + * Stephan Aßmus */ #include @@ -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; - const char* fTitle; - const char* fMsgId; + char* fAppName; + char* fTitle; + char* fMsgId; float fProgress; - int32 fTimeout; - const char* fIconFile; + bigtime_t fTimeout; + char* fIconFile; entry_ref fFileRef; - const char* fMessage; - const char* fApp; + char* fMessage; + 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((void*)fAppName); - if (fTitle) - free((void*)fTitle); - if (fMsgId) - free((void*)fMsgId); - if (fIconFile) - free((void*)fIconFile); - if (fMessage) - free((void*)fMessage); - if (fApp) - free((void*)fApp); + free(fAppName); + free(fTitle); + free(fMsgId); + free(fIconFile); + free(fMessage); + free(fApp); - int32 i; - void* item; - - for (i = 0; item = fRefs->ItemAt(i); i++) - delete item; + for (int32 i = 0; void* item = fRefs->ItemAt(i); i++) + delete (BEntry*)item; delete fRefs; - for (i = 0; item = fArgv->ItemAt(i); i++) { - if (item != NULL) - free(item); - } + for (int32 i = 0; void* item = fArgv->ItemAt(i); i++) + 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) { - fTimeout = atol(argument); - } else if (strcmp(option, "icon") == 0) { + else if (strcmp(option, "timeout") == 0) + fTimeout = atol(argument) * 1000000; + 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()) { - BNotification* msg = new BNotification(fType); - msg->SetApplication(fAppName); - msg->SetTitle(fTitle); - msg->SetContent(fMessage); + if (HasGoodArguments()) { + BNotification notification(fType); + notification.SetApplication(fAppName); + notification.SetTitle(fTitle); + notification.SetContent(fMessage); - if (fMsgId) - msg->SetMessageID(fMsgId); + if (fMsgId != NULL) + 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) - msg->SetIcon(bitmap); + if (bitmap) { + notification.SetIcon(bitmap); + delete bitmap; + } } - if (fApp) - msg->SetOnClickApp(fApp); + if (fApp != NULL) + notification.SetOnClickApp(fApp); if (fHasFile) - msg->SetOnClickFile(&fFile); + notification.SetOnClickFile(&fFile); - int32 i; - void* item; - - for (i = 0; item = fRefs->ItemAt(i); i++) { + for (int32 i = 0; void* 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; diff --git a/src/kits/app/Notification.cpp b/src/kits/app/Notification.cpp index e39c927130..07c4dcc1b8 100644 --- a/src/kits/app/Notification.cpp +++ b/src/kits/app/Notification.cpp @@ -4,48 +4,40 @@ * * Authors: * Pier Luigi Fiorini, pierluigi.fiorini@gmail.com + * Stephan Aßmus */ + +#include + +#include + #include #include #include -#include #include -#include 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) - free(fAppName); - if (fTitle) - free(fTitle); - if (fContent) - free(fContent); - if (fID) - free(fID); - if (fApp) - free(fApp); + delete fFile; + delete fBitmap; - delete fRefs; - delete fArgv; + for (int32 i = fRefs.CountItems() - 1; i >= 0; i--) + 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 = NULL; - - if (app) - fAppName = strdup(app); + fAppName = app; } @@ -84,11 +72,7 @@ BNotification::Title() const void BNotification::SetTitle(const char* title) { - free(fTitle); - fTitle = NULL; - - if (title) - fTitle = strdup(title); + fTitle = title; } @@ -102,11 +86,7 @@ BNotification::Content() const void BNotification::SetContent(const char* content) { - free(fContent); - fContent = NULL; - - if (content) - fContent = strdup(content); + fContent = content; } @@ -120,11 +100,7 @@ BNotification::MessageID() const void BNotification::SetMessageID(const char* id) { - free(fID); - fID = NULL; - - if (id) - fID = strdup(id); + fID = id; } @@ -152,65 +128,115 @@ BNotification::OnClickApp() const void BNotification::SetOnClickApp(const char* app) { - free(fApp); - fApp = NULL; - - if (app) - fApp = strdup(app); + fApp = 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* -BNotification::OnClickRefs() const -{ - return fRefs; -} - - -void +status_t BNotification::AddOnClickRef(const entry_ref* ref) { - fRefs->AddItem((void*)ref); + if (ref == NULL) + return B_BAD_VALUE; + + return AddOnClickRef(*ref); } -BList* -BNotification::OnClickArgv() const +int32 +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 -BNotification::SetIcon(BBitmap* icon) +status_t +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; } diff --git a/src/kits/app/Roster.cpp b/src/kits/app/Roster.cpp index 2f14cdae09..9e354cba24 100644 --- a/src/kits/app/Roster.cpp +++ b/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, - that will displays it according to its settings and filters. + The notification is delivered asynchronously to the notification_server, + 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()); - msg.AddString("app", notification->Application()); - msg.AddString("title", notification->Title()); - msg.AddString("content", notification->Content()); + status_t ret = msg.AddInt32("type", (int32)notification.Type()); + if (ret == B_OK) + ret = msg.AddString("app", notification.Application()); + if (ret == B_OK) + ret = msg.AddString("title", notification.Title()); + if (ret == B_OK) + ret = msg.AddString("content", notification.Content()); - if (notification->MessageID()) - msg.AddString("messageID", notification->MessageID()); + if (ret == B_OK && notification.MessageID() != NULL) + ret = msg.AddString("messageID", notification.MessageID()); - if (notification->Type() == B_PROGRESS_NOTIFICATION) - msg.AddFloat("progress", notification->Progress()); + if (ret == B_OK && notification.Type() == B_PROGRESS_NOTIFICATION) + ret = msg.AddFloat("progress", notification.Progress()); - if (notification->OnClickApp()) - msg.AddString("onClickApp", notification->OnClickApp()); - if (notification->OnClickFile()) - msg.AddRef("onClickFile", notification->OnClickFile()); + if (ret == B_OK && notification.OnClickApp() != NULL) + ret = msg.AddString("onClickApp", notification.OnClickApp()); + if (ret == B_OK && notification.OnClickFile() != NULL) + 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(); - for (i = 0; i < refs->CountItems(); i++) - msg.AddRef("onClickRef", (entry_ref*)refs->ItemAt(i)); + if (ret == B_OK) { + for (int32 i = 0; i < notification.CountOnClickArgs(); i++) { + ret = msg.AddString("onClickArgv", notification.OnClickArgAt(i)); + if (ret != B_OK) + break; + } + } - BList* argv = notification->OnClickArgv(); - for (i = 0; i < argv->CountItems(); i++) - msg.AddString("onClickArgv", (const char*)argv->ItemAt(i)); - - BBitmap* icon = notification->Icon(); - - BMessage archive; - if (icon && icon->Archive(&archive) == B_OK) - msg.AddMessage("icon", &archive); + if (ret == B_OK) { + const BBitmap* icon = notification.Icon(); + if (icon != NULL) { + BMessage archive; + ret = icon->Archive(&archive); + if (ret == B_OK) + ret = msg.AddMessage("icon", &archive); + } + } // Custom time out - if (timeout > 0) - msg.AddInt32("timeout", timeout); + if (ret == B_OK && timeout > 0) + ret = msg.AddInt64("timeout", timeout); // Send message - BMessenger server(kNotificationServerSignature); - return server.SendMessage(&msg); + if (ret == B_OK) { + BMessenger server(kNotificationServerSignature); + ret = server.SendMessage(&msg); + } + + return ret; } diff --git a/src/servers/notification/NotificationView.cpp b/src/servers/notification/NotificationView.cpp index 0ac963327b..d223a4d56f 100644 --- a/src/servers/notification/NotificationView.cpp +++ b/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 */ #include @@ -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) - timeout = fParent->Timeout(); - bigtime_t delay = timeout * 1000 * 1000; - - if (delay > 0) - fRunner = new BMessageRunner(BMessenger(Parent()), &msg, delay, 1); + if (fDetails->FindInt64("timeout", &timeout) != B_OK) + timeout = fParent->Timeout() * 1000000; + + if (timeout > 0) + fRunner = new BMessageRunner(BMessenger(Parent()), &msg, timeout, 1); }