Merge pull request #17431 from tchaikov/wip-da-SCA-20170510

Coverity and SCA fixes

Reviewed-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>

src/client/Client.cc

@@ -258,6 +258,7 @@ Client::Client(Messenger *m, MonClient *mc, Objecter *objecter_)
   _reset_faked_inos();
   //
   root = 0;
+  root_ancestor = 0;
 
   num_flushing_caps = 0;
 
@@ -1730,7 +1731,6 @@ int Client::make_request(MetaRequest *request,
     r = request->get_abort_code();
     request->item.remove_myself();
     unregister_request(request);
-    put_request(request); // ours
     return r;
   }
 

src/client/MetaRequest.h

@@ -82,8 +82,8 @@ struct MetaRequest {
     regetattr_mask(0),
     mds(-1), resend_mds(-1), send_to_auth(false), sent_on_mseq(0),
     num_fwd(0), retry_attempt(0),
-    reply(0), 
-    kick(false), success(false),
+    reply(0),
+    kick(false), success(false), dirp(NULL),
     got_unsafe(false), item(this), unsafe_item(this),
     unsafe_dir_item(this), unsafe_target_item(this),
     caller_cond(0), dispatch_cond(0) {

src/common/Timer.h

@@ -57,7 +57,7 @@ class SafeTimer
    * setting safe_callbacks = false eliminates the lock cycle issue.
    * */
   SafeTimer(CephContext *cct, Mutex &l, bool safe_callbacks=true);
-  ~SafeTimer();
+  virtual ~SafeTimer();
 
   /* Call with the event_lock UNLOCKED.
    *

src/crush/CrushCompiler.cc

@@ -1023,8 +1023,13 @@ int CrushCompiler::parse_choose_args(iter_t const& i)
     err << choose_arg_index << " duplicated" << std::endl;
     return -1;
   }
+  const auto max_buckets = crush.get_max_buckets();
+  if (max_buckets < 0) {
+    err << "get_max_buckets() returned error" << std::endl;
+    return -1;
+  }
   crush_choose_arg_map arg_map;
-  arg_map.size = crush.get_max_buckets();
+  arg_map.size = max_buckets;
   arg_map.args = (crush_choose_arg *)calloc(arg_map.size, sizeof(crush_choose_arg));
   for (iter_t p = i->children.begin() + 2; p != i->children.end(); p++) {
     int r = 0;

src/test/librados/aio.cc

@@ -4,6 +4,7 @@
 #include "test/librados/test.h"
 #include "include/types.h"
 #include "include/stringify.h"
+#include "include/scope_guard.h"
 
 #include "gtest/gtest.h"
 #include <errno.h>
@@ -257,6 +258,7 @@ TEST(LibRadosAio, SimpleWrite) {
   ASSERT_EQ("", test_data.init());
   ASSERT_EQ(0, rados_aio_create_completion((void*)&test_data,
 	      set_completion_complete, set_completion_safe, &my_completion));
+  auto sg = make_scope_guard([&] { rados_aio_release(my_completion); });
   char buf[128];
   memset(buf, 0xcc, sizeof(buf));
   ASSERT_EQ(0, rados_aio_write(test_data.m_ioctx, "foo",
@@ -272,6 +274,7 @@ TEST(LibRadosAio, SimpleWrite) {
   rados_completion_t my_completion2;
   ASSERT_EQ(0, rados_aio_create_completion((void*)&test_data,
 	      set_completion_complete, set_completion_safe, &my_completion2));
+  auto sg2 = make_scope_guard([&] { rados_aio_release(my_completion2); });
   ASSERT_EQ(0, rados_aio_write(test_data.m_ioctx, "foo",
 			       my_completion2, buf, sizeof(buf), 0));
   {
@@ -280,8 +283,6 @@ TEST(LibRadosAio, SimpleWrite) {
     sem_wait(test_data.m_sem);
   }
   ASSERT_EQ(0, rados_aio_get_return_value(my_completion2));
-  rados_aio_release(my_completion);
-  rados_aio_release(my_completion2);
 }
 
 TEST(LibRadosAio, SimpleWritePP) {
@@ -2613,6 +2614,7 @@ TEST(LibRadosAioEC, SimpleWrite) {
   ASSERT_EQ("", test_data.init());
   ASSERT_EQ(0, rados_aio_create_completion((void*)&test_data,
 	      set_completion_completeEC, set_completion_safeEC, &my_completion));
+  auto sg = make_scope_guard([&] { rados_aio_release(my_completion); });
   char buf[128];
   memset(buf, 0xcc, sizeof(buf));
   ASSERT_EQ(0, rados_aio_write(test_data.m_ioctx, "foo",
@@ -2628,6 +2630,7 @@ TEST(LibRadosAioEC, SimpleWrite) {
   rados_completion_t my_completion2;
   ASSERT_EQ(0, rados_aio_create_completion((void*)&test_data,
 	      set_completion_completeEC, set_completion_safeEC, &my_completion2));
+  auto sg2 = make_scope_guard([&] { rados_aio_release(my_completion2); });
   ASSERT_EQ(0, rados_aio_write(test_data.m_ioctx, "foo",
 			       my_completion2, buf, sizeof(buf), 0));
   {
@@ -2636,8 +2639,6 @@ TEST(LibRadosAioEC, SimpleWrite) {
     sem_wait(test_data.m_sem);
   }
   ASSERT_EQ(0, rados_aio_get_return_value(my_completion2));
-  rados_aio_release(my_completion);
-  rados_aio_release(my_completion2);
 }
 
 TEST(LibRadosAioEC, SimpleWritePP) {

src/test/librados/c_read_operations.cc

@@ -8,6 +8,7 @@
 #include "include/rados/librados.h"
 #include "test/librados/test.h"
 #include "test/librados/TestCase.h"
+#include "include/scope_guard.h"
 
 const char *data = "testdata";
 const char *obj = "testobj";
@@ -158,10 +159,10 @@ TEST_F(CReadOpsTest, AssertExists) {
 
   rados_completion_t completion;
   ASSERT_EQ(0, rados_aio_create_completion(NULL, NULL, NULL, &completion));
+  auto sg = make_scope_guard([&] { rados_aio_release(completion); });
   ASSERT_EQ(0, rados_aio_read_op_operate(op, ioctx, completion, obj, 0));
   rados_aio_wait_for_complete(completion);
   ASSERT_EQ(-ENOENT, rados_aio_get_return_value(completion));
-  rados_aio_release(completion);
   rados_release_read_op(op);
 
   write_object();

src/test/librados_test_stub/LibradosTestStub.cc

@@ -1148,6 +1148,7 @@ int cls_get_request_origin(cls_method_context_t hctx, entity_inst_t *origin) {
     ctx->io_ctx_impl->get_rados_client();
 
   struct sockaddr_in sin;
+  memset(&sin, 0, sizeof(sin));
   sin.sin_family = AF_INET;
   sin.sin_port = 0;
   inet_pton(AF_INET, "127.0.0.1", &sin.sin_addr);

src/test/test_ipaddr.cc

@@ -36,7 +36,7 @@ TEST(CommonIPAddr, TestNotFound)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -62,7 +62,7 @@ TEST(CommonIPAddr, TestV4_Simple)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -88,7 +88,7 @@ TEST(CommonIPAddr, TestV4_Prefix25)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -114,7 +114,7 @@ TEST(CommonIPAddr, TestV4_Prefix16)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -139,7 +139,7 @@ TEST(CommonIPAddr, TestV4_PrefixTooLong)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = NULL;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -160,7 +160,7 @@ TEST(CommonIPAddr, TestV4_PrefixZero)
   struct sockaddr_in net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -186,8 +186,8 @@ TEST(CommonIPAddr, TestV6_Simple)
   struct sockaddr_in6 net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
-  
+  memset(&net, 0, sizeof(net));
+
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
   one.ifa_name = eth0;
@@ -212,7 +212,7 @@ TEST(CommonIPAddr, TestV6_Prefix57)
   struct sockaddr_in6 net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
@@ -237,7 +237,7 @@ TEST(CommonIPAddr, TestV6_PrefixTooLong)
   struct sockaddr_in6 net;
   const struct ifaddrs *result;
 
-  memset(&net, '0', sizeof(net));
+  memset(&net, 0, sizeof(net));
 
   one.ifa_next = NULL;
   one.ifa_addr = (struct sockaddr*)&a_one;

src/tools/rbd/Utils.cc

@@ -633,12 +633,11 @@ int validate_snapshot_name(at::ArgumentModifier mod,
 int get_image_options(const boost::program_options::variables_map &vm,
 		      bool get_format, librbd::ImageOptions *opts) {
   uint64_t order = 0, stripe_unit = 0, stripe_count = 0, object_size = 0;
-  uint64_t features = 0, features_clear = 0, features_set = 0;
+  uint64_t features = 0, features_clear = 0;
   std::string data_pool;
   bool order_specified = true;
   bool features_specified = false;
   bool features_clear_specified = false;
-  bool features_set_specified = false;
   bool stripe_specified = false;
 
   if (vm.count(at::IMAGE_ORDER)) {
@@ -647,7 +646,7 @@ int get_image_options(const boost::program_options::variables_map &vm,
 	      << std::endl;
   } else if (vm.count(at::IMAGE_OBJECT_SIZE)) {
     object_size = vm[at::IMAGE_OBJECT_SIZE].as<uint64_t>();
-    order = std::round(std::log2(object_size)); 
+    order = std::round(std::log2(object_size));
   } else {
     order_specified = false;
   }
@@ -744,8 +743,6 @@ int get_image_options(const boost::program_options::variables_map &vm,
   if (features_clear_specified) {
     opts->set(RBD_IMAGE_OPTION_FEATURES_CLEAR, features_clear);
   }
-  if (features_set_specified)
-    opts->set(RBD_IMAGE_OPTION_FEATURES_SET, features_set);
   if (stripe_specified) {
     opts->set(RBD_IMAGE_OPTION_STRIPE_UNIT, stripe_unit);
     opts->set(RBD_IMAGE_OPTION_STRIPE_COUNT, stripe_count);