[Libreoffice] PATCHES] Fixed some CLang++ errors/warnings in sal

Hi,

please find attached some changes to OSL. Those are bugs reported by CLang++.

The patches are contributed under the LGPLv3+ / MPL.

Thanks,
Julien

From 7a8e6017129480a6a353dd81764f9c7e572389e2 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Mon, 11 Apr 2011 23:59:34 -0700
Subject: [PATCH 1/5] Added error handling for pthread_mutexattr_settype.

This fixes a dead assignment reported by CLang++.
---
 sal/osl/unx/mutex.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/sal/osl/unx/mutex.c b/sal/osl/unx/mutex.c
index c44b332..3dca138 100644
--- a/sal/osl/unx/mutex.c
+++ b/sal/osl/unx/mutex.c
@@ -72,13 +72,22 @@ oslMutex SAL_CALL osl_createMutex()
     pthread_mutexattr_init(&aMutexAttr);
 
     nRet = pthread_mutexattr_settype(&aMutexAttr, PTHREAD_MUTEX_RECURSIVE);
-    
+    if ( nRet != 0 )
+    {
+        OSL_TRACE("osl_createMutex : mutex settype failed. Errno: %d; %s\n",
+                  nRet, strerror(nRet));
+
+        free(pMutex);
+        pMutex = 0;
+        return (oslMutex) pMutex;
+    }
+
     nRet = pthread_mutex_init(&(pMutex->mutex), &aMutexAttr);
     if ( nRet != 0 )
     {
-        OSL_TRACE("osl_createMutex : mutex init failed. Errno: %d; %s\n",  
+        OSL_TRACE("osl_createMutex : mutex init failed. Errno: %d; %s\n",
                   nRet, strerror(nRet));
-        
+
         free(pMutex);
         pMutex = 0;
     }
-- 
1.7.1

From b62a179285bdae118daa91e6832d023e7ab75512 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Tue, 12 Apr 2011 00:00:47 -0700
Subject: [PATCH 2/5] Added handling for the write errors in receiveFdPipe.

Fixed a dead assignment in process.c reported by CLang++
---
 sal/osl/unx/process.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/sal/osl/unx/process.c b/sal/osl/unx/process.c
index d21b1b7..b41301c 100644
--- a/sal/osl/unx/process.c
+++ b/sal/osl/unx/process.c
@@ -376,6 +376,16 @@ static oslSocket receiveFdPipe(int PipeFD)
     OSL_TRACE("receiveFdPipe : writing back %i",nRetCode);
     nRead=write(PipeFD,&nRetCode,sizeof(nRetCode));
 
+    if ( nRead < 0 )
+    {
+        OSL_TRACE("write failed (%s)", strerror(errno));
+    }
+    else if ( nRead != sizeof(nRetCode) )
+    {
+        // TODO: Handle this case.
+        OSL_TRACE("partial write: wrote %d out of %d)", nRead, sizeof(nRetCode));
+    }
+
 #if defined(IOCHANNEL_TRANSFER_BSD_RENO)
     free(cmptr);
 #endif
-- 
1.7.1

From c9464cb57b9b82afd47eb723463863def7039098 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Tue, 12 Apr 2011 06:40:24 -0700
Subject: [PATCH 3/5] Fixed a potential null-dereferencing error in osl_closeProfile

Store the new profile in a temporary variable and assign
it to the old profile after we have checked if it is null.
Seen with CLang++.
---
 sal/osl/unx/profile.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/sal/osl/unx/profile.c b/sal/osl/unx/profile.c
index 7c7b04c..c56c8bb 100644
--- a/sal/osl/unx/profile.c
+++ b/sal/osl/unx/profile.c
@@ -274,6 +274,7 @@ static oslProfile SAL_CALL osl_psz_openProfile(const sal_Char *pszProfileName, o
 sal_Bool SAL_CALL osl_closeProfile(oslProfile Profile)
 {
     osl_TProfileImpl* pProfile = (osl_TProfileImpl*)Profile;
+    osl_TProfileImpl* pTmpProfile;
 
 #ifdef TRACE_OSL_PROFILE
     OSL_TRACE("In  osl_closeProfile\n");
@@ -303,22 +304,22 @@ sal_Bool SAL_CALL osl_closeProfile(oslProfile Profile)
 
     if ( ! ( pProfile->m_Flags & osl_Profile_READLOCK ) && ( pProfile->m_Flags & FLG_MODIFIED ) )
     {
-        pProfile = acquireProfile(Profile,sal_True);
+        pTmpProfile = acquireProfile(Profile,sal_True);
 
-        if ( pProfile != 0 )
+        if ( pTmpProfile != 0 )
         {
-            sal_Bool bRet = storeProfile(pProfile, sal_True);
+            sal_Bool bRet = storeProfile(pTmpProfile, sal_True);
             OSL_ASSERT(bRet);
             (void)bRet;
         }
     }
     else
     {
-        pProfile = acquireProfile(Profile,sal_False);
+        pTmpProfile = acquireProfile(Profile,sal_False);
     }
 
 
-    if ( pProfile == 0 )
+    if ( pTmpProfile == 0 )
     {
         pthread_mutex_unlock(&(pProfile->m_AccessLock));
 #ifdef TRACE_OSL_PROFILE
@@ -327,6 +328,8 @@ sal_Bool SAL_CALL osl_closeProfile(oslProfile Profile)
         return sal_False;
     }
 
+    pProfile = pTmpProfile;
+
     if (pProfile->m_pFile != NULL)
         closeFileImpl(pProfile->m_pFile,pProfile->m_Flags);
 
-- 
1.7.1

From 555980b0a7a0a5c01cc9bba9085175b121995df6 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Tue, 12 Apr 2011 06:56:47 -0700
Subject: [PATCH 4/5] Fixed some false positives 'dead assignments' seen in CLang++

We removed the OSL_DEBUG_LEVEL > 1 code and replace them with OSL_TRACE.
This should provide the same coverage and remove CLang++ complains.
---
 sal/osl/unx/pipe.c   |    4 +---
 sal/osl/unx/socket.c |   12 +++---------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/sal/osl/unx/pipe.c b/sal/osl/unx/pipe.c
index 38a1413..1ae6fb4 100644
--- a/sal/osl/unx/pipe.c
+++ b/sal/osl/unx/pipe.c
@@ -359,12 +359,10 @@ void SAL_CALL osl_closePipe( oslPipe pPipe )
         len = sizeof(addr);
 
         nRet = connect( fd, (struct sockaddr *)&addr, len);
-#if OSL_DEBUG_LEVEL > 1
         if ( nRet < 0 )
         {
-            perror("connect in osl_destroyPipe");
+            OSL_TRACE("connect in osl_destroyPipe failed with error: %s", strerror(errno));
         }
-#endif /* OSL_DEBUG_LEVEL */
         close(fd);
     }
 #endif /* LINUX */
diff --git a/sal/osl/unx/socket.c b/sal/osl/unx/socket.c
index 3f31779..b9fb92e 100644
--- a/sal/osl/unx/socket.c
+++ b/sal/osl/unx/socket.c
@@ -1705,12 +1705,10 @@ void SAL_CALL osl_closeSocket(oslSocket pSocket)
         socklen_t nSockLen = sizeof(s.aSockAddr);
 
         nRet = getsockname(nFD, &s.aSockAddr, &nSockLen);
-#if OSL_DEBUG_LEVEL > 1
         if ( nRet < 0 )
         {
-            perror("getsockname");
+            OSL_TRACE("getsockname call failed with error: %s", strerror(errno));
         }
-#endif /* OSL_DEBUG_LEVEL */
 
         if ( s.aSockAddr.sa_family == AF_INET )
         {
@@ -1720,20 +1718,16 @@ void SAL_CALL osl_closeSocket(oslSocket pSocket)
             }
 
             nConnFD = socket(AF_INET, SOCK_STREAM, 0);
-#if OSL_DEBUG_LEVEL > 1
             if ( nConnFD < 0 )
             {
-                perror("socket");
+                OSL_TRACE("socket call failed with error: %s", strerror(errno));
             }
-#endif /* OSL_DEBUG_LEVEL */
 
             nRet = connect(nConnFD, &s.aSockAddr, sizeof(s.aSockAddr));
-#if OSL_DEBUG_LEVEL > 1
             if ( nRet < 0 )
             {
-                perror("connect");
+                OSL_TRACE("connect call failed with error: %s", strerror(errno));
             }
-#endif /* OSL_DEBUG_LEVEL */
             close(nConnFD);
         }
         pSocket->m_bIsAccepting = sal_False;
-- 
1.7.1

From 8b1abbe1d1f38882ba93084f7fec378f0cc40875 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Tue, 12 Apr 2011 07:59:00 -0700
Subject: [PATCH 5/5] No need to check out execv return value.

If we ever return from execv, it is an error and the code already handle that.
This fixes a dead assignment warning under CLang++.
---
 sal/osl/unx/process.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/sal/osl/unx/process.c b/sal/osl/unx/process.c
index b41301c..7d65b54 100644
--- a/sal/osl/unx/process.c
+++ b/sal/osl/unx/process.c
@@ -544,8 +544,9 @@ static void ChildStatusProc(void *pData)
                 if (stdError[1] != -1) close( stdError[1] );
             }
 
-            pid=execv(data.m_pszArgs[0], (sal_Char **)data.m_pszArgs);
-
+            // No need to check the return value of execv. If we return from
+            // it, an error has occurred.
+            execv(data.m_pszArgs[0], (sal_Char **)data.m_pszArgs);
         }
 
         OSL_TRACE("Failed to exec, errno=%d (%s)\n", errno, strerror(errno));
-- 
1.7.1