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.chaffr...@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.chaffr...@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.chaffr...@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.chaffr...@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.chaffr...@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
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice