minor improvements for QSharedMemory

remove unused includes;
generate error string if error occurred only;
make some error strings equal on different platforms;
avoid code duplication;
minor code simlifications and clean-ups

Merge-request: 1248
Reviewed-by: Harald Fernengel <harald.fernengel@nokia.com>

5 files changed, 81 insertions, 80 deletions

diff --git a/src/corelib/kernel/qsharedmemory.cpp b/src/corelib/kernel/qsharedmemory.cpp
index acb6044..9ab9fa4 100644
--- a/src/corelib/kernel/qsharedmemory.cpp
+++ b/src/corelib/kernel/qsharedmemory.cpp
@@ -247,7 +247,7 @@ void QSharedMemory::setNativeKey(const QString &key)
     if (isAttached())
         detach();
     d->cleanHandle();
-    d->key = QString();
+    d->key.clear();
     d->nativeKey = key;
 }
 
@@ -285,7 +285,7 @@ bool QSharedMemoryPrivate::initKey()
         return false;
     }
 #endif
-    errorString = QString();
+    errorString.clear();
     error = QSharedMemory::NoError;
     return true;
 }
@@ -342,28 +342,24 @@ bool QSharedMemory::create(int size, AccessMode mode)
     if (!d->initKey())
         return false;
 
+    if (size <= 0) {
+        d->error = QSharedMemory::InvalidSize;
+        d->errorString = QSharedMemory::tr("%1: create size is less then 0").arg(QLatin1String("QSharedMemory::create"));
+        return false;
+    }
+
 #ifndef QT_NO_SYSTEMSEMAPHORE
 #ifndef Q_OS_WIN
     // Take ownership and force set initialValue because the semaphore
     // might have already existed from a previous crash.
     d->systemSemaphore.setKey(d->key, 1, QSystemSemaphore::Create);
 #endif
-#endif
 
-    QString function = QLatin1String("QSharedMemory::create");
-#ifndef QT_NO_SYSTEMSEMAPHORE
     QSharedMemoryLocker lock(this);
-    if (!d->key.isNull() && !d->tryLocker(&lock, function))
+    if (!d->key.isNull() && !d->tryLocker(&lock, QLatin1String("QSharedMemory::create")))
         return false;
 #endif
 
-    if (size <= 0) {
-        d->error = QSharedMemory::InvalidSize;
-        d->errorString =
-	    QSharedMemory::tr("%1: create size is less then 0").arg(function);
-        return false;
-    }
-
     if (!d->create(size))
         return false;
 
diff --git a/src/corelib/kernel/qsharedmemory_p.h b/src/corelib/kernel/qsharedmemory_p.h
index d5fafef..182876b 100644
--- a/src/corelib/kernel/qsharedmemory_p.h
+++ b/src/corelib/kernel/qsharedmemory_p.h
@@ -74,8 +74,6 @@ namespace QSharedMemoryPrivate
 #elif defined(Q_OS_SYMBIAN)
 #include <e32std.h>
 #include <sys/types.h>
-#else
-#include <sys/sem.h>
 #endif
 
 QT_BEGIN_NAMESPACE
@@ -151,7 +149,7 @@ public:
 #endif
 
 #ifndef QT_NO_SYSTEMSEMAPHORE
-    bool tryLocker(QSharedMemoryLocker *locker, const QString function) {
+    inline bool tryLocker(QSharedMemoryLocker *locker, const QString &function) {
         if (!locker->lock()) {
             errorString = QSharedMemory::tr("%1: unable to lock").arg(function);
             error = QSharedMemory::LockError;
diff --git a/src/corelib/kernel/qsharedmemory_symbian.cpp b/src/corelib/kernel/qsharedmemory_symbian.cpp
index fdd513a..3430aaa 100644
--- a/src/corelib/kernel/qsharedmemory_symbian.cpp
+++ b/src/corelib/kernel/qsharedmemory_symbian.cpp
@@ -41,19 +41,21 @@
 
 #include "qsharedmemory.h"
 #include "qsharedmemory_p.h"
-#include "qsystemsemaphore.h"
+
 #include "qcore_symbian_p.h"
 #include <qdebug.h>
 
-QT_BEGIN_NAMESPACE
-
 #ifndef QT_NO_SHAREDMEMORY
 
 #define QSHAREDMEMORY_DEBUG
 
-QSharedMemoryPrivate::QSharedMemoryPrivate() : QObjectPrivate(),
-        memory(0), size(0), error(QSharedMemory::NoError),
-           systemSemaphore(QString()), lockedByMe(false)
+QT_BEGIN_NAMESPACE
+
+QSharedMemoryPrivate::QSharedMemoryPrivate()
+    : QObjectPrivate(), memory(0), size(0), error(QSharedMemory::NoError),
+#ifndef QT_NO_SYSTEMSEMAPHORE
+      systemSemaphore(QString()), lockedByMe(false)
+#endif
 {
 }
 
@@ -61,6 +63,7 @@ void QSharedMemoryPrivate::setErrorString(const QString &function, TInt errorCod
 {
     if (errorCode == KErrNone)
         return;
+
     switch (errorCode) {
     case KErrAlreadyExists:
         error = QSharedMemory::AlreadyExists;
@@ -88,11 +91,19 @@ void QSharedMemoryPrivate::setErrorString(const QString &function, TInt errorCod
 #if defined QSHAREDMEMORY_DEBUG
         qDebug() << errorString << "key" << key;
 #endif
+        break;
     }
 }
 
 key_t QSharedMemoryPrivate::handle()
 {
+    // don't allow making handles on empty keys
+    if (nativeKey.isEmpty()) {
+        error = QSharedMemory::KeyError;
+        errorString = QSharedMemory::tr("%1: key is empty").arg(QLatin1String("QSharedMemory::handle"));
+        return 0;
+    }
+
     // Not really cost effective to check here if shared memory is attachable, as it requires
     // exactly the same call as attaching, so always assume handle is valid and return failure
     // from attach.
@@ -107,21 +118,17 @@ bool QSharedMemoryPrivate::cleanHandle()
 
 bool QSharedMemoryPrivate::create(int size)
 {
-    QString function = QLatin1String("QSharedMemory::create");
-    if (nativeKey.isEmpty()) {
-        error = QSharedMemory::KeyError;
-        errorString = QSharedMemory::tr("%1: key error").arg(function);
+    if (!handle())
         return false;
-    }
 
     TPtrC ptr(qt_QString2TPtrC(nativeKey));
 
     TInt err = chunk.CreateGlobal(ptr, size, size);
 
-    setErrorString(function, err);
-
-    if (err != KErrNone)
+    if (err != KErrNone) {
+        setErrorString(QLatin1String("QSharedMemory::create"), err);
         return false;
+    }
 
     // Zero out the created chunk
     Mem::FillZ(chunk.Base(), chunk.Size());
@@ -133,12 +140,8 @@ bool QSharedMemoryPrivate::attach(QSharedMemory::AccessMode /* mode */)
 {
     // Grab a pointer to the memory block
     if (!chunk.Handle()) {
-        QString function = QLatin1String("QSharedMemory::handle");
-        if (nativeKey.isEmpty()) {
-            error = QSharedMemory::KeyError;
-            errorString = QSharedMemory::tr("%1: unable to make key").arg(function);
+        if (!handle())
             return false;
-        }
 
         TPtrC ptr(qt_QString2TPtrC(nativeKey));
 
@@ -147,7 +150,7 @@ bool QSharedMemoryPrivate::attach(QSharedMemory::AccessMode /* mode */)
         err = chunk.OpenGlobal(ptr, false);
 
         if (err != KErrNone) {
-            setErrorString(function, err);
+            setErrorString(QLatin1String("QSharedMemory::attach"), err);
             return false;
         }
     }
@@ -168,6 +171,6 @@ bool QSharedMemoryPrivate::detach()
     return true;
 }
 
-#endif //QT_NO_SHAREDMEMORY
-
 QT_END_NAMESPACE
+
+#endif //QT_NO_SHAREDMEMORY
diff --git a/src/corelib/kernel/qsharedmemory_unix.cpp b/src/corelib/kernel/qsharedmemory_unix.cpp
index 286a1b8..ae199c4 100644
--- a/src/corelib/kernel/qsharedmemory_unix.cpp
+++ b/src/corelib/kernel/qsharedmemory_unix.cpp
@@ -43,25 +43,26 @@
 
 #include "qsharedmemory.h"
 #include "qsharedmemory_p.h"
-#include "qsystemsemaphore.h"
-#include <qdir.h>
-#include <qdebug.h>
 
-#include <errno.h>
+#include <qdebug.h>
+#include <qfile.h>
 
 #ifndef QT_NO_SHAREDMEMORY
 #include <sys/types.h>
 #include <sys/ipc.h>
 #include <sys/shm.h>
-#include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
-#endif //QT_NO_SHAREDMEMORY
+#endif // QT_NO_SHAREDMEMORY
+#include <errno.h>
 
 #include "private/qcore_unix_p.h"
 
 #ifndef QT_NO_SHAREDMEMORY
+
+//#define QSHAREDMEMORY_DEBUG
+
 QT_BEGIN_NAMESPACE
 
 QSharedMemoryPrivate::QSharedMemoryPrivate()
@@ -78,6 +79,7 @@ void QSharedMemoryPrivate::setErrorString(const QString &function)
     // EINVAL is handled in functions so they can give better error strings
     switch (errno) {
     case EACCES:
+    case EPERM:
         errorString = QSharedMemory::tr("%1: permission denied").arg(function);
         error = QSharedMemory::PermissionDenied;
         break;
@@ -98,9 +100,10 @@ void QSharedMemoryPrivate::setErrorString(const QString &function)
     default:
         errorString = QSharedMemory::tr("%1: unknown error %2").arg(function).arg(errno);
         error = QSharedMemory::UnknownError;
-#if defined QSHAREDMEMORY_DEBUG
+#ifdef QSHAREDMEMORY_DEBUG
         qDebug() << errorString << "key" << key << "errno" << errno << EINVAL;
 #endif
+        break;
     }
 }
 
@@ -117,21 +120,21 @@ key_t QSharedMemoryPrivate::handle()
 
     // don't allow making handles on empty keys
     if (nativeKey.isEmpty()) {
-        errorString = QSharedMemory::tr("%1: key is empty").arg(QLatin1String("QSharedMemory::handle:"));
+        errorString = QSharedMemory::tr("%1: key is empty").arg(QLatin1String("QSharedMemory::handle"));
         error = QSharedMemory::KeyError;
         return 0;
     }
 
     // ftok requires that an actual file exists somewhere
     if (!QFile::exists(nativeKey)) {
-        errorString = QSharedMemory::tr("%1: UNIX key file doesn't exist").arg(QLatin1String("QSharedMemory::handle:"));
+        errorString = QSharedMemory::tr("%1: UNIX key file doesn't exist").arg(QLatin1String("QSharedMemory::handle"));
         error = QSharedMemory::NotFound;
         return 0;
     }
 
     unix_key = ftok(QFile::encodeName(nativeKey).constData(), 'Q');
     if (-1 == unix_key) {
-        errorString = QSharedMemory::tr("%1: ftok failed").arg(QLatin1String("QSharedMemory::handle:"));
+        errorString = QSharedMemory::tr("%1: ftok failed").arg(QLatin1String("QSharedMemory::handle"));
         error = QSharedMemory::KeyError;
         unix_key = 0;
     }
@@ -149,14 +152,14 @@ key_t QSharedMemoryPrivate::handle()
     -1 error
      0 already existed
      1 created
-  */
+*/
 int QSharedMemoryPrivate::createUnixKeyFile(const QString &fileName)
 {
     if (QFile::exists(fileName))
         return 0;
 
     int fd = qt_safe_open(QFile::encodeName(fileName).constData(),
-            O_EXCL | O_CREAT | O_RDWR, 0640);
+                          O_EXCL | O_CREAT | O_RDWR, 0640);
     if (-1 == fd) {
         if (errno == EEXIST)
             return 0;
@@ -179,16 +182,13 @@ bool QSharedMemoryPrivate::cleanHandle()
 bool QSharedMemoryPrivate::create(int size)
 {
     // build file if needed
-    bool createdFile = false;
     int built = createUnixKeyFile(nativeKey);
     if (built == -1) {
-        errorString = QSharedMemory::tr("%1: unable to make key").arg(QLatin1String("QSharedMemory::handle:"));
+        errorString = QSharedMemory::tr("%1: unable to make key").arg(QLatin1String("QSharedMemory::create"));
         error = QSharedMemory::KeyError;
         return false;
     }
-    if (built == 1) {
-        createdFile = true;
-    }
+    bool createdFile = built == 1;
 
     // get handle
     if (!handle()) {
@@ -202,7 +202,7 @@ bool QSharedMemoryPrivate::create(int size)
         QString function = QLatin1String("QSharedMemory::create");
         switch (errno) {
         case EINVAL:
-            errorString = QSharedMemory::tr("%1: system-imposed size restrictions").arg(QLatin1String("QSharedMemory::handle"));
+            errorString = QSharedMemory::tr("%1: system-imposed size restrictions").arg(function);
             error = QSharedMemory::InvalidSize;
             break;
         default:
@@ -280,7 +280,7 @@ bool QSharedMemoryPrivate::detach()
     if (shmid_ds.shm_nattch == 0) {
         // mark for removal
         if (-1 == shmctl(id, IPC_RMID, &shmid_ds)) {
-            setErrorString(QLatin1String("QSharedMemory::remove"));
+            setErrorString(QLatin1String("QSharedMemory::detach"));
             switch (errno) {
             case EINVAL:
                 return true;
@@ -296,7 +296,6 @@ bool QSharedMemoryPrivate::detach()
     return true;
 }
 
-
 QT_END_NAMESPACE
 
 #endif // QT_NO_SHAREDMEMORY
diff --git a/src/corelib/kernel/qsharedmemory_win.cpp b/src/corelib/kernel/qsharedmemory_win.cpp
index 3cc14fe..d4da30d 100644
--- a/src/corelib/kernel/qsharedmemory_win.cpp
+++ b/src/corelib/kernel/qsharedmemory_win.cpp
@@ -41,24 +41,30 @@
 
 #include "qsharedmemory.h"
 #include "qsharedmemory_p.h"
-#include "qsystemsemaphore.h"
-#include <qdebug.h>
 
-QT_BEGIN_NAMESPACE
+#include <qdebug.h>
 
 #ifndef QT_NO_SHAREDMEMORY
 
-QSharedMemoryPrivate::QSharedMemoryPrivate() : QObjectPrivate(),
-        memory(0), size(0), error(QSharedMemory::NoError),
-           systemSemaphore(QString()), lockedByMe(false), hand(0)
+//#define QSHAREDMEMORY_DEBUG
+
+QT_BEGIN_NAMESPACE
+
+QSharedMemoryPrivate::QSharedMemoryPrivate()
+    : QObjectPrivate(), memory(0), size(0), error(QSharedMemory::NoError),
+#ifndef QT_NO_SYSTEMSEMAPHORE
+      systemSemaphore(QString()), lockedByMe(false),
+#endif
+      hand(0)
 {
 }
 
 void QSharedMemoryPrivate::setErrorString(const QString &function)
 {
-    BOOL windowsError = GetLastError();
+    DWORD windowsError = GetLastError();
     if (windowsError == 0)
         return;
+
     switch (windowsError) {
     case ERROR_ALREADY_EXISTS:
         error = QSharedMemory::AlreadyExists;
@@ -89,19 +95,20 @@ void QSharedMemoryPrivate::setErrorString(const QString &function)
     default:
         errorString = QSharedMemory::tr("%1: unknown error %2").arg(function).arg(windowsError);
         error = QSharedMemory::UnknownError;
-#if defined QSHAREDMEMORY_DEBUG
+#ifdef QSHAREDMEMORY_DEBUG
         qDebug() << errorString << "key" << key;
 #endif
+        break;
     }
 }
 
 HANDLE QSharedMemoryPrivate::handle()
 {
     if (!hand) {
-        QString function = QLatin1String("QSharedMemory::handle");
+        // don't allow making handles on empty keys
         if (nativeKey.isEmpty()) {
             error = QSharedMemory::KeyError;
-            errorString = QSharedMemory::tr("%1: unable to make key").arg(function);
+            errorString = QSharedMemory::tr("%1: key is empty").arg(QLatin1String("QSharedMemory::handle"));
             return false;
         }
 #ifndef Q_OS_WINCE
@@ -111,11 +118,10 @@ HANDLE QSharedMemoryPrivate::handle()
         // attach as it seems.
         hand = CreateFileMapping(INVALID_HANDLE_VALUE, 0, PAGE_READWRITE, 0, 0, (wchar_t*)nativeKey.utf16());
 #endif
-        if (!hand) {
-            setErrorString(function);
-            return false;
-        }
+        if (!hand)
+            setErrorString(QLatin1String("QSharedMemory::handle"));
     }
+
     return hand;
 }
 
@@ -132,20 +138,20 @@ bool QSharedMemoryPrivate::cleanHandle()
 
 bool QSharedMemoryPrivate::create(int size)
 {
-    QString function = QLatin1String("QSharedMemory::create");
     if (nativeKey.isEmpty()) {
         error = QSharedMemory::KeyError;
-        errorString = QSharedMemory::tr("%1: key error").arg(function);
+        errorString = QSharedMemory::tr("%1: key is empty").arg(QLatin1String("QSharedMemory::create"));
         return false;
     }
 
     // Create the file mapping.
     hand = CreateFileMapping(INVALID_HANDLE_VALUE, 0, PAGE_READWRITE, 0, size, (wchar_t*)nativeKey.utf16());
-    setErrorString(function);
 
     // hand is valid when it already exists unlike unix so explicitly check
-    if (error == QSharedMemory::AlreadyExists || !hand)
+    if (error == QSharedMemory::AlreadyExists || !hand) {
+        setErrorString(QLatin1String("QSharedMemory::create"));
         return false;
+    }
 
     return true;
 }
@@ -167,7 +173,7 @@ bool QSharedMemoryPrivate::attach(QSharedMemory::AccessMode mode)
         // Windows doesn't set an error code on this one,
         // it should only be a kernel memory error.
         error = QSharedMemory::UnknownError;
-        errorString = QSharedMemory::tr("%1: size query failed").arg(QLatin1String("QSharedMemory::attach: "));
+        errorString = QSharedMemory::tr("%1: size query failed").arg(QLatin1String("QSharedMemory::attach"));
         return false;
     }
     size = info.RegionSize;
@@ -189,7 +195,6 @@ bool QSharedMemoryPrivate::detach()
     return cleanHandle();
 }
 
-#endif //QT_NO_SHAREDMEMORY
-
-
 QT_END_NAMESPACE
+
+#endif // QT_NO_SHAREDMEMORY