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

[Julien Chaffraix]

>> Applied - would you want to look into the TODO case as well? A rather
>> nasty cause for errors ...
>
> While it is theoretically possible to hit this, I doubt in practice
> that writing an int to a file descriptor will ever do a partial write.
> It may just be better to change that to an OSL_ASSERT instead of
> adding a for loop. Thoughts?

I just went ahead and fixed the root cause of the bug instead of using
an assertion. While at it, I fixed another error in the same file.

The patch is released under the dual LGPLv3 (or later) / MPL license.

Thanks,
Julien

From 7ce65499e18e68f83c2e45515ed37bd964847a74 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffr...@gmail.com>
Date: Sat, 16 Apr 2011 17:42:15 -0700
Subject: [PATCH] Fixed write(2) usage in process.c.

Shared the code in a new function (safeWrite) that can handle partial write.
Fixed the 2 usages of write in the file.
---
 sal/osl/unx/process.c |   35 +++++++++++++++++++++--------------
 1 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/sal/osl/unx/process.c b/sal/osl/unx/process.c
index 7d65b54..207e023 100644
--- a/sal/osl/unx/process.c
+++ b/sal/osl/unx/process.c
@@ -300,6 +300,23 @@ static sal_Bool sendFdPipe(int PipeFD, int SocketFD)
     return bRet;
 }
 
+static sal_Bool safeWrite(int socket, void* data, sal_uInt32 dataSize)
+{
+    sal_Int32 nToWrite = dataSize;
+    // Check for overflow as we convert a signed to an unsigned.
+    OSL_ASSERT(dataSize == (sal_uInt32)nToWrite);
+    while ( nToWrite ) {
+        sal_Int32 nWritten = write(socket, data, nToWrite);
+        if ( nWritten < 0 )
+            return sal_False;
+
+        OSL_ASSERT(nWritten > 0);
+        nToWrite -= nWritten;
+    }
+
+    return sal_True;
+}
+
 /**********************************************
  receiveFdPipe
  *********************************************/
@@ -374,17 +391,8 @@ static oslSocket receiveFdPipe(int PipeFD)
     }
 
     OSL_TRACE("receiveFdPipe : writing back %i",nRetCode);
-    nRead=write(PipeFD,&nRetCode,sizeof(nRetCode));
-
-    if ( nRead < 0 )
-    {
+    if ( !safeWrite(PipeFD, &nRetCode, sizeof(nRetCode)) )
         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);
@@ -476,7 +484,6 @@ static void ChildStatusProc(void *pData)
     {
         /* Child */
         int chstatus = 0;
-        sal_Int32 nWrote;
 
         if (channel[0] != -1) close(channel[0]);
 
@@ -554,11 +561,11 @@ static void ChildStatusProc(void *pData)
         OSL_TRACE("ChildStatusProc : starting '%s' failed",data.m_pszArgs[0]);
         
         /* if we reach here, something went wrong */
-        nWrote = write(channel[1], &errno, sizeof(errno));
-        if (nWrote != sizeof(errno))
+        if ( !safeWrite(channel[1], &errno, sizeof(errno)) )
             OSL_TRACE("sendFdPipe : sending failed (%s)",strerror(errno));
 
-        if (channel[1] != -1) close(channel[1]);
+        if ( channel[1] != -1 )
+            close(channel[1]);
 
         _exit(255);
     }
-- 
1.7.1


_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice