From: Kefu Chai <kchai@redhat.com>
Date: Fri, 22 Sep 2017 07:14:09 +0000 (+0800)
Subject: test/objectstore/test_bluefs: refactor related to open_for_write()
X-Git-Tag: v13.0.1~434^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f69631a2f985030da729b6a92fdf69f666bcd074;p=ceph.git

test/objectstore/test_bluefs: refactor related to open_for_write()

and avoid the false positive of coverity warning:

 Use after free (USE_AFTER_FREE)

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/test/objectstore/test_bluefs.cc b/src/test/objectstore/test_bluefs.cc
index bd6f9ae22913..7dc2e8e6086f 100644
--- a/src/test/objectstore/test_bluefs.cc
+++ b/src/test/objectstore/test_bluefs.cc
@@ -11,6 +11,7 @@
 #include "global/global_init.h"
 #include "common/ceph_argparse.h"
 #include "include/stringify.h"
+#include "include/scope_guard.h"
 #include "common/errno.h"
 #include <gtest/gtest.h>
 
@@ -137,7 +138,6 @@ TEST(BlueFS, small_appends) {
 
 void write_data(BlueFS &fs, uint64_t rationed_bytes)
 {
-    BlueFS::FileWriter *h;
     int j=0, r=0;
     uint64_t written_bytes = 0;
     rationed_bytes -= ALLOC_SIZE;
@@ -151,7 +151,10 @@ void write_data(BlueFS &fs, uint64_t rationed_bytes)
     while (1) {
       string file = "file.";
       file.append(to_string(j));
+      BlueFS::FileWriter *h;
       ASSERT_EQ(0, fs.open_for_write(dir, file, &h, false));
+      ASSERT_NE(nullptr, h);
+      auto sg = make_scope_guard([&fs, h] { fs.close_writer(h); });
       bufferlist bl;
       char *buf = gen_buffer(ALLOC_SIZE);
       bufferptr bp = buffer::claim_char(ALLOC_SIZE, buf);
@@ -159,11 +162,9 @@ void write_data(BlueFS &fs, uint64_t rationed_bytes)
       h->append(bl.c_str(), bl.length());
       r = fs.fsync(h);
       if (r < 0) {
-         fs.close_writer(h);
          break;
       }
       written_bytes += g_conf->bluefs_alloc_size;
-      fs.close_writer(h);
       j++;
       if ((rationed_bytes - written_bytes) <= g_conf->bluefs_alloc_size) {
         break;
@@ -190,27 +191,26 @@ void create_single_file(BlueFS &fs)
 
 void write_single_file(BlueFS &fs, uint64_t rationed_bytes)
 {
-    BlueFS::FileWriter *h;
     stringstream ss;
-    string dir = "dir.test";
-    string file = "testfile";
-    int r=0;
+    const string dir = "dir.test";
+    const string file = "testfile";
     uint64_t written_bytes = 0;
     rationed_bytes -= ALLOC_SIZE;
     while (1) {
+      BlueFS::FileWriter *h;
       ASSERT_EQ(0, fs.open_for_write(dir, file, &h, false));
+      ASSERT_NE(nullptr, h);
+      auto sg = make_scope_guard([&fs, h] { fs.close_writer(h); });
       bufferlist bl;
       char *buf = gen_buffer(ALLOC_SIZE);
       bufferptr bp = buffer::claim_char(ALLOC_SIZE, buf);
       bl.push_back(bp);
       h->append(bl.c_str(), bl.length());
-      r = fs.fsync(h);
+      int r = fs.fsync(h);
       if (r < 0) {
-         fs.close_writer(h);
          break;
       }
       written_bytes += g_conf->bluefs_alloc_size;
-      fs.close_writer(h);
       if ((rationed_bytes - written_bytes) <= g_conf->bluefs_alloc_size) {
         break;
       }
@@ -358,7 +358,6 @@ TEST(BlueFS, test_simple_compaction_sync) {
   ASSERT_EQ(0, fs.mkfs(fsid));
   ASSERT_EQ(0, fs.mount());
   {
-    BlueFS::FileWriter *h;
     for (int i=0; i<10; i++) {
        string dir = "dir.";
        dir.append(to_string(i));
@@ -366,14 +365,16 @@ TEST(BlueFS, test_simple_compaction_sync) {
        for (int j=0; j<10; j++) {
           string file = "file.";
 	  file.append(to_string(j));
+          BlueFS::FileWriter *h;
           ASSERT_EQ(0, fs.open_for_write(dir, file, &h, false));
+          ASSERT_NE(nullptr, h);
+          auto sg = make_scope_guard([&fs, h] { fs.close_writer(h); });
           bufferlist bl;
           char *buf = gen_buffer(4096);
 	  bufferptr bp = buffer::claim_char(4096, buf);
 	  bl.push_back(bp);
           h->append(bl.c_str(), bl.length());
           fs.fsync(h);
-          fs.close_writer(h);
        }
     }
   }
@@ -410,7 +411,6 @@ TEST(BlueFS, test_simple_compaction_async) {
   ASSERT_EQ(0, fs.mkfs(fsid));
   ASSERT_EQ(0, fs.mount());
   {
-    BlueFS::FileWriter *h;
     for (int i=0; i<10; i++) {
        string dir = "dir.";
        dir.append(to_string(i));
@@ -418,14 +418,16 @@ TEST(BlueFS, test_simple_compaction_async) {
        for (int j=0; j<10; j++) {
           string file = "file.";
 	  file.append(to_string(j));
+          BlueFS::FileWriter *h;
           ASSERT_EQ(0, fs.open_for_write(dir, file, &h, false));
+          ASSERT_NE(nullptr, h);
+          auto sg = make_scope_guard([&fs, h] { fs.close_writer(h); });
           bufferlist bl;
           char *buf = gen_buffer(4096);
 	  bufferptr bp = buffer::claim_char(4096, buf);
 	  bl.push_back(bp);
           h->append(bl.c_str(), bl.length());
           fs.fsync(h);
-          fs.close_writer(h);
        }
     }
   }