Add scope_guard for ad-hoc resource management (#2313)

References #2261.  Suggested by:
https://stackoverflow.com/questions/10270328/the-simplest-and-neatest-c11-scopeguard

src/fdcache.cpp

@@ -935,6 +935,7 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_CRIT_HEAD, "Could not open cache file");
                 continue;
             }
+            scope_guard guard([&]() { close(cache_file_fd); });
 
             // get inode number for cache file
             struct stat st;
@@ -943,7 +944,6 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_FILE_PROB, object_file_path.c_str(), strOpenedWarn.c_str());
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_CRIT_HEAD, "Could not get file inode number for cache file");
 
-                close(cache_file_fd);
                 continue;
             }
             ino_t cache_file_inode = st.st_ino;
@@ -956,7 +956,6 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_FILE_PROB, object_file_path.c_str(), strOpenedWarn.c_str());
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_CRIT_HEAD, "Could not load cache file stats information");
 
-                close(cache_file_fd);
                 continue;
             }
             cfstat.Release();
@@ -967,7 +966,6 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_FILE_PROB, object_file_path.c_str(), strOpenedWarn.c_str());
                 S3FS_PRN_CACHE(fp, CACHEDBG_FMT_CRIT_HEAD2 "The cache file size(%lld) and the value(%lld) from cache file stats are different", static_cast<long long int>(st.st_size), static_cast<long long int>(pagelist.Size()));
 
-                close(cache_file_fd);
                 continue;
             }
 
@@ -999,7 +997,6 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
             }
             err_area_list.clear();
             warn_area_list.clear();
-            close(cache_file_fd);
         }
     }
     closedir(statsdir);

src/fdcache_fdinfo.cpp

@@ -28,6 +28,7 @@
 
 #include "common.h"
 #include "s3fs_logger.h"
+#include "s3fs_util.h"
 #include "fdcache_fdinfo.h"
 #include "fdcache_pseudofd.h"
 #include "fdcache_entity.h"
@@ -179,19 +180,25 @@ bool PseudoFdInfo::OpenUploadFd(AutoLock::Type type)
     }
 
     // duplicate fd
-    if(-1 == (upload_fd = dup(physical_fd)) || 0 != lseek(upload_fd, 0, SEEK_SET)){
+    int fd;
+    if(-1 == (fd = dup(physical_fd))){
         S3FS_PRN_ERR("Could not duplicate physical file descriptor(errno=%d)", errno);
-        if(-1 != upload_fd){
-            close(upload_fd);
-        }
+        return false;
+    }
+    scope_guard guard([&]() { close(fd); });
+
+    if(0 != lseek(fd, 0, SEEK_SET)){
+        S3FS_PRN_ERR("Could not seek physical file descriptor(errno=%d)", errno);
         return false;
     }
     struct stat st;
-    if(-1 == fstat(upload_fd, &st)){
+    if(-1 == fstat(fd, &st)){
         S3FS_PRN_ERR("Invalid file descriptor for uploading(errno=%d)", errno);
-        close(upload_fd);
         return false;
     }
+
+    guard.dismiss();
+    upload_fd = fd;
     return true;
 }
 

src/fdcache_stat.cpp

@@ -208,34 +208,35 @@ bool CacheFileStat::RawOpen(bool readonly)
         return false;
     }
     // open
+    int tmpfd;
     if(readonly){
-        if(-1 == (fd = open(sfile_path.c_str(), O_RDONLY))){
+        if(-1 == (tmpfd = open(sfile_path.c_str(), O_RDONLY))){
             S3FS_PRN_ERR("failed to read only open cache stat file path(%s) - errno(%d)", path.c_str(), errno);
             return false;
         }
     }else{
-        if(-1 == (fd = open(sfile_path.c_str(), O_CREAT|O_RDWR, 0600))){
+        if(-1 == (tmpfd = open(sfile_path.c_str(), O_CREAT|O_RDWR, 0600))){
             S3FS_PRN_ERR("failed to open cache stat file path(%s) - errno(%d)", path.c_str(), errno);
             return false;
         }
     }
+    scope_guard guard([&]() { close(tmpfd); });
+
     // lock
-    if(-1 == flock(fd, LOCK_EX)){
+    if(-1 == flock(tmpfd, LOCK_EX)){
         S3FS_PRN_ERR("failed to lock cache stat file(%s) - errno(%d)", path.c_str(), errno);
-        close(fd);
-        fd = -1;
         return false;
     }
     // seek top
-    if(0 != lseek(fd, 0, SEEK_SET)){
+    if(0 != lseek(tmpfd, 0, SEEK_SET)){
         S3FS_PRN_ERR("failed to lseek cache stat file(%s) - errno(%d)", path.c_str(), errno);
-        flock(fd, LOCK_UN);
-        close(fd);
-        fd = -1;
+        flock(tmpfd, LOCK_UN);
         return false;
     }
     S3FS_PRN_DBG("file locked(%s - %s)", path.c_str(), sfile_path.c_str());
 
+    guard.dismiss();
+    fd = tmpfd;
     return true;
 }
 

src/s3fs_util.h

@@ -21,6 +21,8 @@
 #ifndef S3FS_S3FS_UTIL_H_
 #define S3FS_S3FS_UTIL_H_
 
+#include <functional>
+
 #ifndef CLOCK_REALTIME
 #define CLOCK_REALTIME          0
 #endif
@@ -77,6 +79,32 @@ std::string s3fs_str_realtime();
 // Wrap fclose since it is illegal to take the address of a stdlib function
 int s3fs_fclose(FILE* fp);
 
+class scope_guard {
+public:
+    template<class Callable>
+    explicit scope_guard(Callable&& undo_func)
+        : func(std::forward<Callable>(undo_func))
+    {}
+
+    ~scope_guard() {
+        if(func != nullptr) {
+            func();
+        }
+    }
+
+    void dismiss() {
+        func = nullptr;
+    }
+
+    scope_guard(const scope_guard&) = delete;
+    scope_guard(scope_guard&& other) = delete;
+    scope_guard& operator=(const scope_guard&) = delete;
+    scope_guard& operator=(scope_guard&&) = delete;
+
+private:
+    std::function<void()> func;
+};
+
 #endif // S3FS_S3FS_UTIL_H_
 
 /*