RROW-1500: [C++] Do not ignore return value from truncate in MemoryM…

…appedFile::Create Author: Amir Malekpour <a.malekpour@gmail.com>
apache · Sep 20, 2017 · 15c5231 · 15c5231
1 parent e1d9c7f
commit 15c5231
Showing 1 changed file with 21 additions and 8 deletions.
diff --git a/cpp/src/arrow/io/file.cc b/cpp/src/arrow/io/file.cc
@@ -115,12 +115,13 @@ namespace fs = boost::filesystem;
 namespace arrow {
 namespace io {
 
-static inline Status CheckOpenResult(int ret, int errno_actual,
-                                     const fs::path& file_name) {
+static inline Status CheckFileOpResult(int ret, int errno_actual,
+                                       const fs::path& file_name,
+                                       const std::string& opname) {
   if (ret == -1) {
-    // TODO: errno codes to strings
     std::stringstream ss;
-    ss << "Failed to open local file: " << file_name.string();
+    ss << "Failed to " << opname << " file: " << file_name.string();
+    ss << " , error: " << std::strerror(errno_actual);
     return Status::IOError(ss.str());
   }
   return Status::OK();
@@ -149,7 +150,7 @@ static inline Status FileOpenReadable(const fs::path& file_name, int* fd) {
   errno_actual = errno;
 #endif
 
-  return CheckOpenResult(ret, errno_actual, file_name);
+  return CheckFileOpResult(ret, errno_actual, file_name, "open local");
 }
 
 static inline Status FileOpenWriteable(const fs::path& file_name, bool write_only,
@@ -192,7 +193,7 @@ static inline Status FileOpenWriteable(const fs::path& file_name, bool write_onl
 
   ret = *fd = open(file_name.c_str(), oflag, ARROW_WRITE_SHMODE);
 #endif
-  return CheckOpenResult(ret, errno_actual, file_name);
+  return CheckFileOpResult(ret, errno_actual, file_name, "open local");
 }
 
 static inline Status FileTell(int fd, int64_t* pos) {
@@ -580,14 +581,26 @@ MemoryMappedFile::~MemoryMappedFile() {}
 
 Status MemoryMappedFile::Create(const std::string& path, int64_t size,
                                 std::shared_ptr<MemoryMappedFile>* out) {
+  int ret;
+  errno_t errno_actual;
   std::shared_ptr<FileOutputStream> file;
   RETURN_NOT_OK(FileOutputStream::Open(path, &file));
+
 #ifdef _MSC_VER
-  _chsize_s(file->file_descriptor(), static_cast<size_t>(size));
+  errno_actual = _chsize_s(file->file_descriptor(), static_cast<size_t>(size));
+  ret = errno_actual == 0 ? 0 : -1; 
 #else
-  ftruncate(file->file_descriptor(), static_cast<size_t>(size));
+  ret = ftruncate(file->file_descriptor(), static_cast<size_t>(size));
+  errno_actual = errno;
 #endif
+
+  auto truncate_result = CheckFileOpResult(ret, errno_actual, path, "truncate");
+
+  // First try to close the file before checking the result of 
+  // truncate, so we don't leak the file handle, if truncate failed
   RETURN_NOT_OK(file->Close());
+  RETURN_NOT_OK(truncate_result);
+
   return MemoryMappedFile::Open(path, FileMode::READWRITE, out);
 }