BlackBerry: Revised error handling in event dispatcher

Added an upper bound check for socket notifier file descriptors. Too
high FDs have been a real source of failure.
Added compiler hints to allow error-free code path run faster (errors
are highly unlikely) and adjusted warning messages (some were misleading
and too long).

Change-Id: I1c9c41f5d006ca9d3a28214c3a464555b8a1c71f
Reviewed-by: Thomas McGuire <thomas.mcguire@kdab.com>

1 files changed, 42 insertions, 40 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_blackberry.cpp b/src/corelib/kernel/qeventdispatcher_blackberry.cpp
index 5206334c09..5807683cc8 100644
--- a/src/corelib/kernel/qeventdispatcher_blackberry.cpp
+++ b/src/corelib/kernel/qeventdispatcher_blackberry.cpp
@@ -126,16 +126,16 @@ static int bpsIOHandler(int fd, int io_events, void *data)
         // create unblock event
         bps_event_t *event;
         int result = bps_event_create(&event, bpsUnblockDomain, 0, NULL, NULL);
-        if (result != BPS_SUCCESS) {
-            qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_event_create() failed");
+        if (Q_UNLIKELY(result != BPS_SUCCESS)) {
+            qWarning("QEventDispatcherBlackberry: bps_event_create failed");
             return BPS_FAILURE;
         }
 
         // post unblock event to our thread; in this callback the bps channel is
         // guaranteed to be the same that was active when bps_add_fd was called
         result = bps_push_event(event);
-        if (result != BPS_SUCCESS) {
-            qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_push_event() failed");
+        if (Q_UNLIKELY(result != BPS_SUCCESS)) {
+            qWarning("QEventDispatcherBlackberry: bps_push_event failed");
             bps_event_destroy(event);
             return BPS_FAILURE;
         }
@@ -149,16 +149,16 @@ QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberryPrivate()
 {
     // prepare to use BPS
     int result = bps_initialize();
-    if (result != BPS_SUCCESS)
-        qFatal("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_initialize() failed");
+    if (Q_UNLIKELY(result != BPS_SUCCESS))
+        qFatal("QEventDispatcherBlackberry: bps_initialize failed");
 
     bps_channel = bps_channel_get_active();
 
     // get domain for IO ready and wake up events - ignoring race condition here for now
     if (bpsUnblockDomain == -1) {
         bpsUnblockDomain = bps_register_domain();
-        if (bpsUnblockDomain == -1)
-            qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_register_domain() failed");
+        if (Q_UNLIKELY(bpsUnblockDomain == -1))
+            qWarning("QEventDispatcherBlackberry: bps_register_domain failed");
     }
 }
 
@@ -200,21 +200,26 @@ void QEventDispatcherBlackberry::registerSocketNotifier(QSocketNotifier *notifie
     Q_ASSERT(notifier);
     Q_D(QEventDispatcherBlackberry);
 
-    BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
-
-    // Register the fd with bps
     int sockfd = notifier->socket();
     int type = notifier->type();
-    qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
 
-    int io_events = ioEvents(sockfd);
+    qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
 
-    if (io_events)
-        bps_remove_fd(sockfd);
+    if (Q_UNLIKELY(sockfd >= FD_SETSIZE)) {
+        qWarning() << "QEventDispatcherBlackberry: cannot register QSocketNotifier (fd too high)"
+                   << sockfd;
+        return;
+    }
 
     // Call the base Unix implementation. Needed to allow select() to be called correctly
     QEventDispatcherUNIX::registerSocketNotifier(notifier);
 
+    // Register the fd with bps
+    BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
+    int io_events = ioEvents(sockfd);
+    if (io_events)
+        bps_remove_fd(sockfd);
+
     switch (type) {
     case QSocketNotifier::Read:
         qEventDispatcherDebug << "Registering" << sockfd << "for Reads";
@@ -231,44 +236,41 @@ void QEventDispatcherBlackberry::registerSocketNotifier(QSocketNotifier *notifie
         break;
     }
 
-    errno = 0;
-    int result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data());
-
-    if (result != BPS_SUCCESS)
-        qWarning() << Q_FUNC_INFO << "bps_add_fd() failed" << strerror(errno) << "code:" << errno;
+    const int result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data());
+    if (Q_UNLIKELY(result != BPS_SUCCESS))
+        qWarning() << "QEventDispatcherBlackberry: bps_add_fd failed";
 }
 
 void QEventDispatcherBlackberry::unregisterSocketNotifier(QSocketNotifier *notifier)
 {
     Q_D(QEventDispatcherBlackberry);
 
-    BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
+    int sockfd = notifier->socket();
+
+    qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
+
+    if (Q_UNLIKELY(sockfd >= FD_SETSIZE)) {
+        qWarning() << "QEventDispatcherBlackberry: cannot unregister QSocketNotifier" << sockfd;
+        return;
+    }
 
     // Allow the base Unix implementation to unregister the fd too
     QEventDispatcherUNIX::unregisterSocketNotifier(notifier);
 
     // Unregister the fd with bps
-    int sockfd = notifier->socket();
-    qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
-
+    BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
     const int io_events = ioEvents(sockfd);
-
     int result = bps_remove_fd(sockfd);
-    if (result != BPS_SUCCESS)
-        qWarning() << Q_FUNC_INFO << "bps_remove_fd() failed" << sockfd;
-
+    if (Q_UNLIKELY(result != BPS_SUCCESS))
+        qWarning() << "QEventDispatcherBlackberry: bps_remove_fd failed" << sockfd;
 
-    /* if no other socket notifier is
-     * watching sockfd, our job ends here
-     */
+    // if no other socket notifier is watching sockfd, our job ends here
     if (!io_events)
         return;
 
-    errno = 0;
     result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data());
-    if (result != BPS_SUCCESS) {
-        qWarning() << Q_FUNC_INFO << "bps_add_fd() failed" << strerror(errno) << "code:" << errno;
-    }
+    if (Q_UNLIKELY(result != BPS_SUCCESS))
+        qWarning("QEventDispatcherBlackberry: bps_add_fd error");
 }
 
 static inline int timespecToMillisecs(const timespec &tv)
@@ -352,8 +354,8 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
         // Wait for event or file to be ready
         event = 0;
         const int result = bps_get_event(&event, timeoutLeft);
-        if (result != BPS_SUCCESS)
-            qWarning("QEventDispatcherBlackberry::select: bps_get_event() failed");
+        if (Q_UNLIKELY(result != BPS_SUCCESS))
+            qWarning("QEventDispatcherBlackberry bps_get_event failed");
 
         if (!event)    // In case of !event, we break out of the loop to let Qt process the timers
             break;     // (since timeout has expired) and socket notifiers that are now ready.
@@ -385,13 +387,13 @@ void QEventDispatcherBlackberry::wakeUp()
     Q_D(QEventDispatcherBlackberry);
     if (d->wakeUps.testAndSetAcquire(0, 1)) {
         bps_event_t *event;
-        if (bps_event_create(&event, bpsUnblockDomain, 0, 0, 0) == BPS_SUCCESS) {
-            if (bps_channel_push_event(d->bps_channel, event) == BPS_SUCCESS)
+        if (Q_LIKELY(bps_event_create(&event, bpsUnblockDomain, 0, 0, 0) == BPS_SUCCESS)) {
+            if (Q_LIKELY(bps_channel_push_event(d->bps_channel, event) == BPS_SUCCESS))
                 return;
             else
                 bps_event_destroy(event);
         }
-        qWarning("QEventDispatcherBlackberryPrivate::wakeUp failed");
+        qWarning("QEventDispatcherBlackberry: wakeUp failed");
     }
 }