[Libreoffice] More read(2) / write(2) cleanup in OSL/unx

Hi,

following some of my previous changes, here are more change to the way
we use write(2) and read(2).

The changes are made available under LGPLv3+/MPL.

Thanks,
Julien

From 1020d181fdc5b326497ddbbac21457f16b4e7e55 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Sat, 23 Apr 2011 07:26:44 -0700
Subject: [PATCH 1/4] Refactored the safeWrite method in a new file.

This enables other files to use it, which will be added in a follow-up step.
---
 sal/osl/unx/makefile.mk        |    6 +++-
 sal/osl/unx/process.c          |   18 +-------------
 sal/osl/unx/readwrite_helper.c |   50 ++++++++++++++++++++++++++++++++++++++++
 sal/osl/unx/readwrite_helper.h |   32 +++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 19 deletions(-)
 create mode 100644 sal/osl/unx/readwrite_helper.c
 create mode 100644 sal/osl/unx/readwrite_helper.h

diff --git a/sal/osl/unx/makefile.mk b/sal/osl/unx/makefile.mk
index 6fb6768..4a1c113 100644
--- a/sal/osl/unx/makefile.mk
+++ b/sal/osl/unx/makefile.mk
@@ -78,7 +78,8 @@ SLOFILES= \
             $(SLO)$/file_volume.obj \
             $(SLO)$/uunxapi.obj\
             $(SLO)$/process_impl.obj\
-            $(SLO)$/salinit.obj
+            $(SLO)$/salinit.obj \
+            $(SLO)$/readwrite_helper.obj
 
 OBJFILES=   $(OBJ)$/conditn.obj  \
             $(OBJ)$/diagnose.obj \
@@ -107,7 +108,8 @@ OBJFILES=   $(OBJ)$/conditn.obj  \
             $(OBJ)$/file_volume.obj \
             $(OBJ)$/uunxapi.obj\
             $(OBJ)$/process_impl.obj\
-            $(OBJ)$/salinit.obj
+            $(OBJ)$/salinit.obj \
+            $(OBJ)$/readwrite_helper.obj
             
 
 .IF "$(OS)"=="MACOSX"
diff --git a/sal/osl/unx/process.c b/sal/osl/unx/process.c
index aa7d3a8..41848b9 100644
--- a/sal/osl/unx/process.c
+++ b/sal/osl/unx/process.c
@@ -64,6 +64,7 @@
 #include <grp.h>
 
 #include "procimpl.h"
+#include "readwrite_helper.h"
 #include "sockimpl.h"
 #include "secimpl.h"
 
@@ -300,23 +301,6 @@ 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
  *********************************************/
diff --git a/sal/osl/unx/readwrite_helper.c b/sal/osl/unx/readwrite_helper.c
new file mode 100644
index 0000000..4777d08
--- /dev/null
+++ b/sal/osl/unx/readwrite_helper.c
@@ -0,0 +1,50 @@
+/*
+ * Version: MPL 1.1 / GPLv3+ / LGPLv3+
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License or as specified alternatively below. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Initial Developer of the Original Code is
+ *       Julien Chaffraix <julien.chaffraix@gmail.com>
+ * Portions created by the Initial Developer are Copyright (C) 2011 the
+ * Initial Developer. All Rights Reserved.
+ *
+ * Major Contributor(s):
+ *
+ * For minor contributions see the git repository.
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 3 or later (the "GPLv3+"), or
+ * the GNU Lesser General Public License Version 3 or later (the "LGPLv3+"),
+ * in which case the provisions of the GPLv3+ or the LGPLv3+ are applicable
+ * instead of those above.
+ */
+
+#include "readwrite_helper.h"
+
+#include <osl/diagnose.h>
+#include <system.h>
+
+sal_Bool safeWrite(int fd, 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(fd, data, nToWrite);
+        if ( nWritten < 0 )
+            return sal_False;
+
+        OSL_ASSERT(nWritten > 0);
+        nToWrite -= nWritten;
+    }
+
+    return sal_True;
+}
diff --git a/sal/osl/unx/readwrite_helper.h b/sal/osl/unx/readwrite_helper.h
new file mode 100644
index 0000000..dd16f58
--- /dev/null
+++ b/sal/osl/unx/readwrite_helper.h
@@ -0,0 +1,32 @@
+/*
+ * Version: MPL 1.1 / GPLv3+ / LGPLv3+
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License or as specified alternatively below. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Initial Developer of the Original Code is
+ *       Julien Chaffraix <julien.chaffraix@gmail.com>
+ * Portions created by the Initial Developer are Copyright (C) 2011 the
+ * Initial Developer. All Rights Reserved.
+ *
+ * Major Contributor(s):
+ *
+ * For minor contributions see the git repository.
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 3 or later (the "GPLv3+"), or
+ * the GNU Lesser General Public License Version 3 or later (the "LGPLv3+"),
+ * in which case the provisions of the GPLv3+ or the LGPLv3+ are applicable
+ * instead of those above.
+ */
+
+#include <sal/types.h>
+
+sal_Bool safeWrite( int fd, void* data, sal_uInt32 dataSize );
-- 
1.7.1

From 866fcca6775f93d69166996822a653d8bdcb6a85 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Sat, 23 Apr 2011 07:38:27 -0700
Subject: [PATCH 2/4] Switched a write to safeWrite in profile.c.

Cleaned up some code related to a now unneeded assertion.
---
 sal/osl/unx/profile.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/sal/osl/unx/profile.c b/sal/osl/unx/profile.c
index 7c7b04c..f6bdd40 100644
--- a/sal/osl/unx/profile.c
+++ b/sal/osl/unx/profile.c
@@ -437,7 +437,6 @@ sal_Bool SAL_CALL osl_flushProfile(oslProfile Profile)
 
 static sal_Bool writeProfileImpl(osl_TFile* pFile)
 {
-    int BytesWritten=0;
 #if OSL_DEBUG_LEVEL > 1
     unsigned int nLen=0;
 #endif
@@ -459,19 +458,12 @@ static sal_Bool writeProfileImpl(osl_TFile* pFile)
     OSL_ASSERT(nLen == (pFile->m_nWriteBufLen - pFile->m_nWriteBufFree));
 #endif
 
-    BytesWritten = write(pFile->m_Handle, pFile->m_pWriteBuf, pFile->m_nWriteBufLen - 
pFile->m_nWriteBufFree);
-
-    if ( BytesWritten <= 0 )
+    if ( !safeWrite(pFile->m_Handle, pFile->m_pWriteBuf, pFile->m_nWriteBufLen - 
pFile->m_nWriteBufFree) )
     {
         OSL_TRACE("write failed '%s'\n",strerror(errno));
         return (sal_False);
     }
 
-#if OSL_DEBUG_LEVEL > 1
-    OSL_ASSERT(
-        BytesWritten >= 0 && SAL_INT_CAST(unsigned int, BytesWritten) == nLen);
-#endif
-
     free(pFile->m_pWriteBuf);
     pFile->m_pWriteBuf=0;
     pFile->m_nWriteBufLen=0;
-- 
1.7.1

From 6d8ff8ebbf86e47a92c1537c4659fceeff0f5133 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Sat, 23 Apr 2011 07:53:54 -0700
Subject: [PATCH 3/4] Introduced safeRead.

Same function as safeWrite. Converted some methods in process.c to use it.
This usually simplifies the code and add better error checking.
---
 sal/osl/unx/process.c          |   10 +++++-----
 sal/osl/unx/readwrite_helper.c |   26 ++++++++++++++++++++++++++
 sal/osl/unx/readwrite_helper.h |    5 +++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/sal/osl/unx/process.c b/sal/osl/unx/process.c
index 41848b9..bc868ed 100644
--- a/sal/osl/unx/process.c
+++ b/sal/osl/unx/process.c
@@ -283,15 +283,15 @@ static sal_Bool sendFdPipe(int PipeFD, int SocketFD)
         OSL_TRACE("sendFdPipe : sending failed (%s)",strerror(errno));
     }
 
-    nSend=read(PipeFD,&RetCode,sizeof(RetCode));
+    bRet = safeRead(PipeFD, &RetCode, sizeof(RetCode));
 
-    if ( nSend > 0 && RetCode == 1 )
+    if ( bRet && RetCode == 1 )
     {
         OSL_TRACE("sendFdPipe : resource was received\n");
     }
     else
     {
-        OSL_TRACE("sendFdPipe : resource wasn't received\n");        
+        OSL_TRACE("sendFdPipe : resource wasn't received (error %s)\n", strerror(errno));
     }    
     
 #if defined(IOCHANNEL_TRANSFER_BSD_RENO)
@@ -1168,7 +1168,7 @@ sal_Bool osl_getProcStat(pid_t pid, struct osl_procStat* procstat)
         char* tmp=0;
         char prstatbuf[512];
         memset(prstatbuf,0,512);
-        bRet = read(fd,prstatbuf,511) == 511;
+        bRet = safeRead(fd, prstatbuf, 511);
 
         close(fd);
         /*printf("%s\n\n",prstatbuf);*/
@@ -1222,7 +1222,7 @@ sal_Bool osl_getProcStatus(pid_t pid, struct osl_procStat* procstat)
         char* tmp=0;
         char prstatusbuf[512];
         memset(prstatusbuf,0,512);
-        bRet = read(fd,prstatusbuf,511) == 511;
+        bRet = safeRead(fd, prstatusbuf, 511);
 
         close(fd);
 
diff --git a/sal/osl/unx/readwrite_helper.c b/sal/osl/unx/readwrite_helper.c
index 4777d08..41aa41e 100644
--- a/sal/osl/unx/readwrite_helper.c
+++ b/sal/osl/unx/readwrite_helper.c
@@ -48,3 +48,29 @@ sal_Bool safeWrite(int fd, void* data, sal_uInt32 dataSize)
 
     return sal_True;
 }
+
+sal_Bool safeRead( int fd, void* buffer, sal_uInt32 count )
+{
+    sal_Int32 nToRead = count;
+    // Check for overflow as we convert a signed to an unsigned.
+    OSL_ASSERT(count == (sal_uInt32)nToRead);
+    while ( nToRead ) {
+        sal_Int32 nRead = read(fd, buffer, nToRead);
+        if ( nRead < 0 ) {
+            // We were interrupted before reading, retry.
+            if (errno == EINTR)
+                continue;
+
+            return sal_False;
+        }
+
+        // If we reach the EOF, we consider this a partial transfer and thus
+        // an error.
+        if ( nRead == 0 )
+            return sal_False;
+
+        nToRead -= nRead;
+    }
+
+    return sal_True;
+}
diff --git a/sal/osl/unx/readwrite_helper.h b/sal/osl/unx/readwrite_helper.h
index dd16f58..494c886 100644
--- a/sal/osl/unx/readwrite_helper.h
+++ b/sal/osl/unx/readwrite_helper.h
@@ -30,3 +30,8 @@
 #include <sal/types.h>
 
 sal_Bool safeWrite( int fd, void* data, sal_uInt32 dataSize );
+
+// This function *will* read |count| bytes from |fd|, busy looping
+// if needed. Don't use it when you don't know if you can request enough
+// data. It will return sal_False for any partial transfer or error.
+sal_Bool safeRead( int fd, void* buffer, sal_uInt32 count );
-- 
1.7.1

From f1210458fab2e465e2e01e035204de729970c0a4 Mon Sep 17 00:00:00 2001
From: Julien Chaffraix <julien.chaffraix@gmail.com>
Date: Sun, 24 Apr 2011 08:37:35 -0700
Subject: [PATCH 4/4] Migrated oslDoCopyFile to using the read/write helpers.

---
 sal/osl/unx/file_misc.cxx      |   20 ++++++++++----------
 sal/osl/unx/readwrite_helper.h |    9 +++++++++
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/sal/osl/unx/file_misc.cxx b/sal/osl/unx/file_misc.cxx
index eb7b94a..51691d5 100644
--- a/sal/osl/unx/file_misc.cxx
+++ b/sal/osl/unx/file_misc.cxx
@@ -39,6 +39,7 @@
 #include "file_path_helper.hxx"
 #include "file_url.h"
 #include "uunxapi.hxx"
+#include "readwrite_helper.h"
 
 #include <sys/types.h>
 #include <errno.h>
@@ -1010,7 +1011,6 @@ static int oslDoCopyFile(const sal_Char* pszSourceFileName, const sal_Char* 
pszD
         return nRet;
     }
 
-    size_t nWritten = 0;
     size_t nRemains = nSourceSize;
    
     if ( nRemains )
@@ -1021,18 +1021,18 @@ static int oslDoCopyFile(const sal_Char* pszSourceFileName, const sal_Char* 
pszD
 
         do
         {
-            nRead = 0;
-            nWritten = 0;
-
             size_t nToRead = std::min( sizeof(pBuffer), nRemains );
-            nRead = read( SourceFileFD, pBuffer, nToRead );
-            if ( (size_t)-1 != nRead )
-                nWritten = write( DestFileFD, pBuffer, nRead );
+            sal_Bool succeeded = safeRead( SourceFileFD, pBuffer, nToRead );
+            if ( !succeeded )
+                break;
+
+            succeeded = safeWrite( DestFileFD, pBuffer, nToRead );
+            if ( !succeeded )
+                break;
 
-            if ( (size_t)-1 != nWritten )
-                nRemains -= nWritten;
+            nRemains -= nToRead;
         }
-        while( nRemains && (size_t)-1 != nRead && nRead == nWritten );
+        while( nRemains );
     }
 
     if ( nRemains )
diff --git a/sal/osl/unx/readwrite_helper.h b/sal/osl/unx/readwrite_helper.h
index 494c886..c96c4e3 100644
--- a/sal/osl/unx/readwrite_helper.h
+++ b/sal/osl/unx/readwrite_helper.h
@@ -29,9 +29,18 @@
 
 #include <sal/types.h>
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 sal_Bool safeWrite( int fd, void* data, sal_uInt32 dataSize );
 
 // This function *will* read |count| bytes from |fd|, busy looping
 // if needed. Don't use it when you don't know if you can request enough
 // data. It will return sal_False for any partial transfer or error.
 sal_Bool safeRead( int fd, void* buffer, sal_uInt32 count );
+
+#ifdef __cplusplus
+}
+#endif
-- 
1.7.1