rgw: refactor D3nDataCache::d3n_io_write to avoid resource leaks

1. D3nChunkDataInfo was allocated at the beginning of the function on
   the heap, and was not freed in case of errors. This memory allocation
   has been moved to where it is being utilised.
2. cache_file file descriptor was not closed if the write to the file
   failed. To avoid such issues, cache_file is declared as a unique_ptr
   with a custom deleter that closes the descriptor when the ptr goes
   out of scope.

Signed-off-by: Vedansh Bhartia <vedanshbhartia@gmail.com>

diff --git a/src/rgw/driver/rados/rgw_d3n_datacache.cc b/src/rgw/driver/rados/rgw_d3n_datacache.cc
index d0ef5e749cac2363d2509c5e0cd0c3cb419fdef7..116a81301081c84dcb1a6baad40eaabd8642f602 100644 (file)
--- a/src/rgw/driver/rados/rgw_d3n_datacache.cc
+++ b/src/rgw/driver/rados/rgw_d3n_datacache.cc
@@ -110,27 +110,37 @@ void D3nDataCache::init(CephContext *_cct) {
 
 int D3nDataCache::d3n_io_write(bufferlist& bl, unsigned int len, std::string oid)
 {
-  D3nChunkDataInfo* chunk_info = new D3nChunkDataInfo;
+  D3nChunkDataInfo* chunk_info{nullptr};
   std::string location = cache_location + url_encode(oid, true);
+  int r = 0;
 
   lsubdout(g_ceph_context, rgw_datacache, 20) << "D3nDataCache: " << __func__ << "(): location=" << location << dendl;
-  FILE *cache_file = nullptr;
-  int r = 0;
-  size_t nbytes = 0;
 
-  cache_file = fopen(location.c_str(), "w+");
-  if (cache_file == nullptr) {
-    ldout(cct, 0) << "ERROR: D3nDataCache::fopen file has return error, errno=" << errno << dendl;
-    return -errno;
-  }
+  {
+    size_t nbytes = 0;
+    auto file_closer = [&r](FILE* fd) {
+      if (fd != nullptr) {
+        r = std::fclose(fd);
+      }
+      fd = nullptr;
+    };
+
+    std::unique_ptr<FILE, decltype(file_closer)> cache_file(
+      fopen(location.c_str(), "w+"), file_closer);
+
+    if (cache_file == nullptr) {
+      ldout(cct, 0) << "ERROR: D3nDataCache::fopen file has return error, errno=" << errno << dendl;
+      return -errno;
+    }
 
-  nbytes = fwrite(bl.c_str(), 1, len, cache_file);
-  if (nbytes != len) {
-    ldout(cct, 0) << "ERROR: D3nDataCache::io_write: fwrite has returned error: nbytes!=len, nbytes=" << nbytes << ", len=" << len << dendl;
-    return -EIO;
+    nbytes = fwrite(bl.c_str(), 1, len, cache_file.get());
+    if (nbytes != len) {
+      ldout(cct, 0) << "ERROR: D3nDataCache::io_write: fwrite has returned error: nbytes!=len, nbytes=" << nbytes << ", len=" << len << dendl;
+      return -EIO;
+    }
   }
 
-  r = fclose(cache_file);
+  // Check whether fclose returned an error
   if (r != 0) {
     ldout(cct, 0) << "ERROR: D3nDataCache::fclsoe file has return error, errno=" << errno << dendl;
     return -errno;
@@ -138,6 +148,7 @@ int D3nDataCache::d3n_io_write(bufferlist& bl, unsigned int len, std::string oid
 
   { // update cahce_map entries for new chunk in cache
     const std::lock_guard l(d3n_cache_lock);
+    chunk_info = new D3nChunkDataInfo;
     chunk_info->oid = oid;
     chunk_info->set_ctx(cct);
     chunk_info->size = len;