- assert result of Close() in destructor of FastOS_File and FastOS_BufferedFile.

- Check result of Close()
- Check result of Sync()
- Scope FastOS_File to avoid explicit Close().
- NULL -> nullptr

5 files changed, 26 insertions, 17 deletions

diff --git a/fastos/src/tests/filetest.cpp b/fastos/src/tests/filetest.cpp
index 09ed1ea33ae..d95580897d0 100644
--- a/fastos/src/tests/filetest.cpp
+++ b/fastos/src/tests/filetest.cpp
@@ -32,7 +32,6 @@ bool createFile(const char* fileName,
                 success = true;
             }
         }
-        cf.Close();
     }
     return success;
 }
@@ -269,8 +268,8 @@ public:
             ssize_t wroteB = file.Write2(buffer, bufSize);
             Progress(wroteB == bufSize, "Writing %d bytes to file", bufSize);
 
-            file.Close();
-
+            bool close_ok = file.Close();
+            assert(close_ok);
             file.enableMemoryMap(mmap_flags);
 
             rc = file.OpenReadOnly();
@@ -323,7 +322,8 @@ public:
             ssize_t wroteB = file.Write2(buffer, bufSize);
             Progress(wroteB == bufSize, "Writing %d bytes to file", bufSize);
 
-            file.Close();
+            bool close_ok = file.Close();
+            assert(close_ok);
 
             if (rc) {
                 file.EnableDirectIO();
@@ -556,7 +556,8 @@ public:
 
         if (myFile->OpenExisting()) {
             Progress(false, "OpenExisting() should not work when '%s' does not exist.", rwFilename.c_str());
-            myFile->Close();
+            bool close_ok = myFile->Close();
+            assert(close_ok);
         } else {
             Progress(true, "OpenExisting() should fail when '%s' does not exist, and it did.", rwFilename.c_str());
         }
@@ -684,8 +685,6 @@ public:
             position = file.GetPosition();
             Progress(position == 4, "File pointer should still be 4 after ReadBuf");
             Progress(strcmp(buffer, "a test") == 0, "[a test]=[%s]", buffer);
-
-            file.Close();
         }
 
         PrintSeparator();
diff --git a/fastos/src/vespa/fastos/file.cpp b/fastos/src/vespa/fastos/file.cpp
index 15a817c0bff..fc7074d7815 100644
--- a/fastos/src/vespa/fastos/file.cpp
+++ b/fastos/src/vespa/fastos/file.cpp
@@ -11,6 +11,7 @@
 #include <cstring>
 #include <fcntl.h>
 #include <cstdlib>
+#include <cassert>
 
 DirectIOException::DirectIOException(const char * fileName, const void * buffer, size_t length, int64_t offset) :
     std::exception(),
@@ -258,8 +259,10 @@ FastOS_FileInterface::CopyFile( const char *src, const char *dst )
                 delete [] tmpBuf;
             } // else out of memory ?
 
-            s.Close();
-            d.Close();
+            bool close_ok = s.Close();
+            assert(close_ok);
+            close_ok = d.Close();
+            assert(close_ok);
         } // else Could not open source or destination file.
     } // else Source file does not exist, or input args are invalid.
 
diff --git a/fastos/src/vespa/fastos/linux_file.cpp b/fastos/src/vespa/fastos/linux_file.cpp
index 6cbd320d426..1074c02b4ac 100644
--- a/fastos/src/vespa/fastos/linux_file.cpp
+++ b/fastos/src/vespa/fastos/linux_file.cpp
@@ -9,13 +9,14 @@
 
 #ifdef __linux__
 #include "file.h"
+#include "file_rw_ops.h"
 #include <sstream>
 #include <unistd.h>
 #include <fcntl.h>
-#include "file_rw_ops.h"
 #include <cstdio>
 #include <cstring>
 #include <system_error>
+#include <cassert>
 
 using fastos::File_RW_Ops;
 
@@ -29,6 +30,11 @@ FastOS_Linux_File::FastOS_Linux_File(const char *filename)
 {
 }
 
+FastOS_Linux_File::~FastOS_Linux_File () {
+    bool ok = Close();
+    assert(ok);
+}
+
 #define DIRECTIOPOSSIBLE(buf, len, off) \
  ((off & (_directIOFileAlign - 1)) == 0 && \
   (len & (_directIOFileAlign - 1)) == 0 && \
@@ -383,12 +389,14 @@ FastOS_Linux_File::Open(unsigned int openFlags, const char *filename)
             if (POSIX_FADV_NORMAL != fadviseOptions) {
                 rc = (posix_fadvise(_filedes, 0, 0, fadviseOptions) == 0);
                 if (!rc) {
-                    Close();
+                    bool close_ok = Close();
+                    assert(close_ok);
                 }
             }
         }
         if (rc) {
-            Sync();
+            bool sync_ok = Sync();
+            assert(sync_ok);
             _cachedSize = GetSize();
             _filePointer = 0;
         }
@@ -397,7 +405,8 @@ FastOS_Linux_File::Open(unsigned int openFlags, const char *filename)
         if (rc && (POSIX_FADV_NORMAL != getFAdviseOptions())) {
             rc = (posix_fadvise(_filedes, 0, 0, getFAdviseOptions()) == 0);
             if (!rc) {
-                Close();
+                bool close_ok = Close();
+                assert(close_ok);
             }
         }
     }
diff --git a/fastos/src/vespa/fastos/linux_file.h b/fastos/src/vespa/fastos/linux_file.h
index 92f0d0b923a..ababf51d26d 100644
--- a/fastos/src/vespa/fastos/linux_file.h
+++ b/fastos/src/vespa/fastos/linux_file.h
@@ -25,9 +25,7 @@ protected:
 
 public:
     FastOS_Linux_File (const char *filename = nullptr);
-    ~FastOS_Linux_File () {
-        Close();
-    }
+    ~FastOS_Linux_File ();
     bool GetDirectIORestrictions(size_t &memoryAlignment, size_t &transferGranularity, size_t &transferMaximum) override;
     bool DirectIOPadding(int64_t offset, size_t length, size_t &padBefore, size_t &padAfter) override;
     void EnableDirectIO() override;
diff --git a/fastos/src/vespa/fastos/unix_file.cpp b/fastos/src/vespa/fastos/unix_file.cpp
index 8dd589d5144..39e31c87702 100644
--- a/fastos/src/vespa/fastos/unix_file.cpp
+++ b/fastos/src/vespa/fastos/unix_file.cpp
@@ -407,7 +407,7 @@ bool FastOS_UNIX_File::Rename (const char *currentFileName, const char *newFileN
 }
 
 bool
-FastOS_UNIX_File::Sync(void)
+FastOS_UNIX_File::Sync()
 {
     assert(IsOpened());