Cleaned up error handling of close system calls. They are now mostly …

common/base/Init.cpp

@@ -372,7 +372,7 @@ void Daemonise() {
   }
 
   for (int fd = 0; fd < maxfd; fd++) {
-    close(fd);
+    close(fd); // ignore errors. Hope for the best. :-)
   }
 
   // send stdout, in and err to /dev/null

common/file/Util.cpp

@@ -137,7 +137,7 @@ bool FindMatchingFiles(const string &directory,
 
   if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
     OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
-    closedir(dp);
+    closedir(dp); // ignore possible error, we've reported an error already. 
     return false;
   }
 
@@ -152,7 +152,7 @@ bool FindMatchingFiles(const string &directory,
     }
     if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
       OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
-      closedir(dp);
+      closedir(dp);// ignore possible error, we've reported an error already. 
       return false;
     }
   }

common/io/Descriptor.cpp

@@ -122,6 +122,8 @@ std::ostream& operator<<(std::ostream &stream, const DescriptorHandle &data) {
 }
 #endif  // _WIN32
 
+
+
 int ToFD(const DescriptorHandle &handle) {
 #ifdef _WIN32
   switch (handle.m_type) {
@@ -186,7 +188,7 @@ bool CreatePipe(DescriptorHandle handle_pair[2]) {
       NULL);
   if (write_handle == INVALID_HANDLE_VALUE) {
     OLA_WARN << "Could not create write end of pipe: %d" << GetLastError();
-    CloseHandle(read_handle);
+    CloseHandle(read_handle); // ignore possible error: already on error path.
     return false;
   }
 
@@ -520,15 +522,19 @@ bool LoopbackDescriptor::Close() {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_handle_pair[0]));
 #else
-    close(m_handle_pair[0]);
+    if (close(m_handle_pair[0])) {
+      OLA_WARN << "LoopbackDescriptor0 close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
   if (m_handle_pair[1] != INVALID_DESCRIPTOR) {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_handle_pair[1]));
 #else
-    close(m_handle_pair[1]);
+    if (close(m_handle_pair[1])) {
+      OLA_WARN << "LoopbackDescriptor1 close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
@@ -542,7 +548,9 @@ bool LoopbackDescriptor::CloseClient() {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_handle_pair[1]));
 #else
-    close(m_handle_pair[1]);
+    if (close(m_handle_pair[1])) {
+      OLA_WARN << "LoopbackDescriptor closeclient close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
@@ -573,8 +581,12 @@ bool PipeDescriptor::Init() {
     CloseHandle(ToHandle(m_in_pair[0]));
     CloseHandle(ToHandle(m_in_pair[1]));
 #else
-    close(m_in_pair[0]);
-    close(m_in_pair[1]);
+    if (close(m_in_pair[0])) {
+      OLA_WARN << "PipeDescriptor0 close: " << strerror(errno);
+    }
+    if (close(m_in_pair[1])) {
+      OLA_WARN << "PipeDescriptor1 close: " << strerror(errno);
+    }
 #endif  // _WIN32
     m_in_pair[0] = m_in_pair[1] = INVALID_DESCRIPTOR;
     return false;
@@ -602,15 +614,19 @@ bool PipeDescriptor::Close() {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_in_pair[0]));
 #else
-    close(m_in_pair[0]);
+    if (close(m_in_pair[0])) {
+      OLA_WARN << "PipeDescriptor0 close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
   if (m_out_pair[1] != INVALID_DESCRIPTOR) {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_out_pair[1]));
 #else
-    close(m_out_pair[1]);
+    if (close(m_out_pair[1])) {
+      OLA_WARN << "PipeDescriptor1 close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
@@ -624,7 +640,9 @@ bool PipeDescriptor::CloseClient() {
 #ifdef _WIN32
     CloseHandle(ToHandle(m_out_pair[1]));
 #else
-    close(m_out_pair[1]);
+    if (close(m_out_pair[1])) {
+      OLA_WARN << "PipeDescriptor closecllient: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 
@@ -680,7 +698,9 @@ bool UnixSocket::Close() {
   return true;
 #else
   if (m_handle != INVALID_DESCRIPTOR) {
-    close(m_handle);
+    if (close(m_handle)) {
+      OLA_WARN << "UnixSocket close: " << strerror(errno);
+    }
   }
 
   m_handle = INVALID_DESCRIPTOR;

common/io/EPoller.cpp

@@ -158,7 +158,9 @@ EPoller::EPoller(ExportMap *export_map, Clock* clock)
 
 EPoller::~EPoller() {
   if (m_epoll_fd != INVALID_DESCRIPTOR) {
-    close(m_epoll_fd);
+    if (close(m_epoll_fd)) {
+      OLA_WARN << "close: " << strerror(errno);
+    }
   }
 
   {

common/io/KQueuePoller.cpp

@@ -106,7 +106,9 @@ KQueuePoller::KQueuePoller(ExportMap *export_map, Clock* clock)
 
 KQueuePoller::~KQueuePoller() {
   if (m_kqueue_fd != INVALID_DESCRIPTOR) {
-    close(m_kqueue_fd);
+    if (close(m_kqueue_fd)) {
+      OLA_WARN << "close: " << strerror(errno);
+    }
   }
 
   {

common/io/Serial.cpp

@@ -75,7 +75,10 @@ bool GetPidFromFile(const string &lock_file, pid_t *pid) {
 
   char buffer[100];
   int r = read(fd, buffer, arraysize(buffer));
-  close(fd);
+  if (close(fd)) {
+    OLA_WARN << "GetPidFromFile close: " << strerror(errno);
+    // if the close failed, we might be able to continue. 
+  }
   if (r < 0) {
     OLA_INFO << "Failed to read PID from " << lock_file << ": "
              << strerror(errno);
@@ -146,7 +149,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
 
   // First, check if the path exists, there's no point trying to open it if not
   if (!FileExists(path)) {
-    OLA_INFO << "Device " << path << " doesn't exist, so there's no point "
+    OLA_DEBUG << "Device " << path << " doesn't exist, so there's no point "
                 "trying to acquire a lock";
     return false;
   }
@@ -202,7 +205,9 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   const string pid_file_contents = str.str();
   size_t r = write(lock_fd, pid_file_contents.c_str(),
                    pid_file_contents.size());
-  close(lock_fd);
+  if (close(lock_fd)) {
+    OLA_WARN << "AcquireUUCPLockAndOpen close: " << strerror(errno);
+  }
   if (r != pid_file_contents.size()) {
     OLA_WARN << "Failed to write complete LCK file: " << lock_file;
     RemoveLockFile(lock_file);
@@ -222,7 +227,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   // further opens.
   if (ioctl(*fd, TIOCEXCL) == -1) {
     OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno);
-    close(*fd);
+    close(*fd); // ignore error: already on error path.
     RemoveLockFile(lock_file);
     return false;
   }

common/network/Socket.cpp

@@ -184,7 +184,7 @@ bool UDPSocket::Close() {
 #else
   if (close(fd)) {
 #endif  // _WIN32
-    OLA_WARN << "close() failed, " << strerror(errno);
+    OLA_WARN << "UDPSocket close() failed, " << strerror(errno);
     return false;
   }
   return true;

common/network/SocketCloser.cpp

@@ -19,6 +19,7 @@
  */
 
 #include "ola/network/SocketCloser.h"
+#include <ola/Logging.h>
 
 #include <errno.h>
 #include <string.h>
@@ -35,7 +36,9 @@ SocketCloser::~SocketCloser() {
 #ifdef _WIN32
     closesocket(m_fd);
 #else
-    close(m_fd);
+    if (close(m_fd)) {
+       OLA_WARN << "socketcloser close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
 }

common/network/TCPConnector.cpp

@@ -83,7 +83,9 @@ void PendingTCPConnection::Close() {
 #ifdef _WIN32
   close(m_handle.m_handle.m_fd);
 #else
-  close(m_handle);
+  if (close(m_handle)) {
+    OLA_WARN << "PendingTCPConnection close: " << strerror(errno);
+  }
 #endif  // _WIN32
 }
 
@@ -145,7 +147,7 @@ TCPConnector::TCPConnectionID TCPConnector::Connect(
       int error = errno;
       OLA_WARN << "connect() to " << endpoint << " returned, "
                << strerror(error);
-      close(sd);
+      close(sd); // we're already on an error return path, don't report followup errors.
       callback->Run(-1, error);
       return 0;
     }

common/network/TCPConnectorTest.cpp

@@ -303,7 +303,9 @@ void TCPConnectorTest::OnConnect(int fd, int error) {
 #ifdef _WIN32
     closesocket(fd);
 #else
-    close(fd);
+    if (close(fd)) {
+      OLA_WARN << "TCPConnectorTest close: " << strerror(errno);
+    }
 #endif  // _WIN32
   }
   m_sucessfull_calls++;

common/network/TCPSocket.cpp

@@ -103,7 +103,9 @@ bool TCPSocket::Close() {
 #ifdef _WIN32
     closesocket(m_handle.m_handle.m_fd);
 #else
-    close(m_handle);
+    if (close(m_handle)) {
+      OLA_WARN << "close: " << strerror(errno);
+    }
 #endif  // _WIN32
     m_handle = ola::io::INVALID_DESCRIPTOR;
   }

common/testing/MockUDPSocket.cpp

@@ -93,7 +93,12 @@ bool MockUDPSocket::Close() {
 #ifdef _WIN32
     closesocket(m_dummy_handle.m_handle.m_fd);
 #else
-    close(m_dummy_handle);
+    if (close(m_dummy_handle)) {
+       OLA_WARN << "close: " << strerror(errno);
+       // XXX What can a caller do if it fails? -- REW
+       // Update: the UDPSocket close function also returns false when it fails. 
+       return false; 
+    }
 #endif  // _WIN32
   }
   return true;

plugins/gpio/GPIODriver.cpp

@@ -148,7 +148,9 @@ bool GPIODriver::SetupGPIO() {
                << strerror(errno);
       failed = true;
     }
-    close(fd);
+    if (close(fd)) {
+       OLA_WARN << "setupGPIO close: " << strerror(errno);
+    }
 
     m_gpio_pins.push_back(pin);
   }
@@ -205,7 +207,9 @@ bool GPIODriver::UpdateGPIOPins(const DmxBuffer &dmx) {
 void GPIODriver::CloseGPIOFDs() {
   GPIOPins::iterator iter = m_gpio_pins.begin();
   for (; iter != m_gpio_pins.end(); ++iter) {
-    close(iter->fd);
+    if (close(iter->fd)) {
+      OLA_WARN << "closeGPIOFDs: " << strerror(errno);
+    }
   }
   m_gpio_pins.clear();
 }

plugins/karate/KaratePlugin.cpp

@@ -62,7 +62,9 @@ bool KaratePlugin::StartHook() {
     // first check if it's there
     int fd;
     if (ola::io::Open(*iter, O_WRONLY, &fd)) {
-      close(fd);
+      if (close(fd)) {
+	OLA_WARN << "close device: " << strerror(errno);
+      }
       KarateDevice *device = new KarateDevice(
           this,
           KARATE_DEVICE_NAME,

plugins/opendmx/OpenDmxThread.cpp

@@ -102,8 +102,10 @@ void *OpenDmxThread::Run() {
         // if you unplug the dongle
         OLA_WARN << "Error writing to device: " << strerror(errno);
 
-        if (close(m_fd) < 0)
-          OLA_WARN << "Close failed " << strerror(errno);
+        if (close(m_fd)) {
+	  // XXX policy throughout is now: don't warn if the close fails on the error path. -- REW
+          OLA_WARN << "OpenDmxThread: close failed: " << strerror(errno);
+	}
         m_fd = INVALID_FD;
       }
     }

plugins/spi/SPIBackend.cpp

@@ -282,7 +282,9 @@ bool HardwareBackend::SetupGPIO() {
                << strerror(errno);
       failed = true;
     }
-    close(fd);
+    if (close(fd)) {
+      OLA_WARN << "HardwareBackend SetupGPIO close: " << strerror(errno);
+    }
   }
 
   if (failed) {
@@ -295,7 +297,9 @@ bool HardwareBackend::SetupGPIO() {
 void HardwareBackend::CloseGPIOFDs() {
   GPIOFds::iterator iter = m_gpio_fds.begin();
   for (; iter != m_gpio_fds.end(); ++iter) {
-    close(*iter);
+    if (close(*iter)) {
+      OLA_WARN << "CloseGPIOFDs: " << strerror(errno);
+    }
   }
   m_gpio_fds.clear();
 }

plugins/spi/SPIWriter.cpp

@@ -69,8 +69,11 @@ SPIWriter::SPIWriter(const string &spi_device,
 }
 
 SPIWriter::~SPIWriter() {
-  if (m_fd >= 0)
-    close(m_fd);
+  if (m_fd >= 0) {
+    if (close(m_fd)) {
+      OLA_WARN << "close: " << strerror(errno);
+    }
+  }
 }
 
 bool SPIWriter::Init() {

plugins/uartdmx/UartDmxPlugin.cpp

@@ -72,7 +72,9 @@ bool UartDmxPlugin::StartHook() {
     }
 
     // can open device, so shut the temporary file descriptor
-    close(fd);
+    if (close(fd)) {  
+      OLA_WARN << "StartHook close: " << strerror(errno);
+    }
     std::auto_ptr<UartDmxDevice> device(new UartDmxDevice(
         this, m_preferences, PLUGIN_NAME, *iter));
 

plugins/uartdmx/UartWidget.cpp

@@ -83,14 +83,14 @@ bool UartWidget::Close() {
     return true;
   }
 
-  if (close(m_fd) > 0) {
+  if (close(m_fd)) {
     OLA_WARN << Name() << " error closing";
     m_fd = NOT_OPEN;
     return false;
-  } else {
-    m_fd = NOT_OPEN;
-    return true;
   }
+
+  m_fd = NOT_OPEN;
+  return true;
 }
 
 bool UartWidget::IsOpen() const {