Validate format strings for CLS_ERR/CLS_LOG

cls_log needed __attribute__((format(printf..)) to allow the compiler
to crosscheck format strings and arguments.  After adding that, there
needed to be a bunch of fixups for %ll, and a few changes for missing
arguments, etc. uncovered by the checking.

Fixes: #3970
Signed-off-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Yehuda Sadeh <yehuda@inktank.com>

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index ad3402e17ba7153be9e03ce5e0ce6bacb48ab338..a55be8c7d836fa7e3d6463c16be656f63964e140 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -216,7 +216,8 @@ int create(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
   }
 
   CLS_LOG(20, "create object_prefix=%s size=%llu order=%u features=%llu",
-         object_prefix.c_str(), size, order, features);
+         object_prefix.c_str(), (unsigned long long)size, order,
+         (unsigned long long)features);
 
   if (features & ~RBD_FEATURES_ALL) {
     return -ENOSYS;
@@ -286,7 +287,7 @@ int get_features(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return -EINVAL;
   }
 
-  CLS_LOG(20, "get_features snap_id=%llu", snap_id);
+  CLS_LOG(20, "get_features snap_id=%llu", (unsigned long long)snap_id);
 
   if (snap_id == CEPH_NOSNAP) {
     int r = read_key(hctx, "features", &features);
@@ -328,7 +329,8 @@ int require_feature(cls_method_context_t hctx, uint64_t need)
   if (r < 0)
     return r;
   if ((features & need) != need) {
-    CLS_LOG(10, "require_feature missing feature %llx, have %llx", need, features);
+    CLS_LOG(10, "require_feature missing feature %llx, have %llx",
+            (unsigned long long)need, (unsigned long long)features);
     return -ENOEXEC;
   }
   return 0;
@@ -355,7 +357,7 @@ int get_size(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return -EINVAL;
   }
 
-  CLS_LOG(20, "get_size snap_id=%llu", snap_id);
+  CLS_LOG(20, "get_size snap_id=%llu", (unsigned long long)snap_id);
 
   int r = read_key(hctx, "order", &order);
   if (r < 0) {
@@ -413,7 +415,8 @@ int set_size(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return r;
   }
 
-  CLS_LOG(20, "set_size size=%llu orig_size=%llu", size);
+  CLS_LOG(20, "set_size size=%llu orig_size=%llu", (unsigned long long)size,
+          (unsigned long long)orig_size);
 
   bufferlist sizebl;
   ::encode(size, sizebl);
@@ -489,7 +492,8 @@ int get_protection_status(cls_method_context_t hctx, bufferlist *in,
   if (r < 0)
     return r;
 
-  CLS_LOG(20, "get_protection_status snap_id=%llu", snap_id.val);
+  CLS_LOG(20, "get_protection_status snap_id=%llu",
+         (unsigned long long)snap_id.val);
 
   if (snap_id == CEPH_NOSNAP)
     return -EINVAL;
@@ -505,7 +509,7 @@ int get_protection_status(cls_method_context_t hctx, bufferlist *in,
 
   if (snap.protection_status >= RBD_PROTECTION_STATUS_LAST) {
     CLS_ERR("invalid protection status for snap id %llu: %u",
-           snap_id.val, snap.protection_status);
+           (unsigned long long)snap_id.val, snap.protection_status);
     return -EIO;
   }
 
@@ -550,14 +554,14 @@ int set_protection_status(cls_method_context_t hctx, bufferlist *in,
   }
 
   CLS_LOG(20, "set_protection_status snapid=%llu status=%u",
-         snap_id.val, status);
+         (unsigned long long)snap_id.val, status);
 
   if (snap_id == CEPH_NOSNAP)
     return -EINVAL;
 
   if (status >= RBD_PROTECTION_STATUS_LAST) {
     CLS_LOG(10, "invalid protection status for snap id %llu: %u",
-           snap_id.val, status);
+           (unsigned long long)snap_id.val, status);
     return -EINVAL;
   }
 
@@ -676,7 +680,8 @@ int set_stripe_unit_count(cls_method_context_t hctx, bufferlist *in, bufferlist
     return r;
   }
   if ((1ull << order) % stripe_unit) {
-    CLS_ERR("stripe unit %lld is not a factor of the object size %lld", stripe_unit, 1ull << order);
+    CLS_ERR("stripe unit %llu is not a factor of the object size %llu",
+            (unsigned long long)stripe_unit, 1ull << order);
     return -EINVAL;
   }
 
@@ -728,7 +733,7 @@ int get_parent(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
   if (r < 0)
     return r;
 
-  CLS_LOG(20, "get_parent snap_id=%llu", snap_id);
+  CLS_LOG(20, "get_parent snap_id=%llu", (unsigned long long)snap_id);
 
   cls_rbd_parent parent;
   r = require_feature(hctx, RBD_FEATURE_LAYERING);
@@ -796,8 +801,9 @@ int set_parent(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return r;
   }
 
-  CLS_LOG(20, "set_parent pool=%lld id=%s snapid=%llu size=%llu",
-         pool, id.c_str(), snapid.val, size);
+  CLS_LOG(20, "set_parent pool=%llu id=%s snapid=%llu size=%llu",
+         (unsigned long long)pool, id.c_str(), (unsigned long long)snapid.val,
+         (unsigned long long)size);
 
   if (pool < 0 || id.length() == 0 || snapid == CEPH_NOSNAP || size == 0) {
     return -EINVAL;
@@ -807,9 +813,10 @@ int set_parent(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
   cls_rbd_parent parent;
   r = read_key(hctx, "parent", &parent);
   if (r == 0) {
-    CLS_LOG(20, "set_parent existing parent pool=%lld id=%s snapid=%llu overlap=%llu",
-           parent.pool, parent.id.c_str(), parent.snapid.val,
-           parent.overlap);
+    CLS_LOG(20, "set_parent existing parent pool=%llu id=%s snapid=%llu"
+            "overlap=%llu", (unsigned long long)parent.pool, parent.id.c_str(),
+           (unsigned long long)parent.snapid.val,
+           (unsigned long long)parent.overlap);
     return -EEXIST;
   }
 
@@ -1162,7 +1169,7 @@ int get_snapshot_name(cls_method_context_t hctx, bufferlist *in, bufferlist *out
     return -EINVAL;
   }
 
-  CLS_LOG(20, "get_snapshot_name snap_id=%llu", snap_id);
+  CLS_LOG(20, "get_snapshot_name snap_id=%llu", (unsigned long long)snap_id);
 
   if (snap_id == CEPH_NOSNAP)
     return -EINVAL;
@@ -1204,7 +1211,8 @@ int snapshot_add(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return -EINVAL;
   }
 
-  CLS_LOG(20, "snapshot_add name=%s id=%llu", snap_meta.name.c_str(), snap_meta.id.val);
+  CLS_LOG(20, "snapshot_add name=%s id=%llu", snap_meta.name.c_str(),
+        (unsigned long long)snap_meta.id.val);
 
   if (snap_meta.id > CEPH_MAXSNAP)
     return -EINVAL;
@@ -1249,13 +1257,14 @@ int snapshot_add(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
        ::decode(old_meta, iter);
       } catch (const buffer::error &err) {
        snapid_t snap_id = snap_id_from_key(it->first);
-       CLS_ERR("error decoding snapshot metadata for snap_id: %llu", snap_id.val);
+       CLS_ERR("error decoding snapshot metadata for snap_id: %llu",
+               (unsigned long long)snap_id.val);
        return -EIO;
       }
       if (snap_meta.name == old_meta.name || snap_meta.id == old_meta.id) {
        CLS_LOG(20, "snap_name %s or snap_id %llu matches existing snap %s %llu",
-               snap_meta.name.c_str(), snap_meta.id.val,
-               old_meta.name.c_str(), old_meta.id.val);
+               snap_meta.name.c_str(), (unsigned long long)snap_meta.id.val,
+               old_meta.name.c_str(), (unsigned long long)old_meta.id.val);
        return -EEXIST;
       }
     }
@@ -1311,7 +1320,7 @@ int snapshot_remove(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return -EINVAL;
   }
 
-  CLS_LOG(20, "snapshot_remove id=%llu", snap_id.val);
+  CLS_LOG(20, "snapshot_remove id=%llu", (unsigned long long)snap_id.val);
 
   // check if the key exists. we can't rely on remove_key doing this for
   // us, since OMAPRMKEYS returns success if the key is not there.
@@ -1989,7 +1998,8 @@ int rbd_assign_bid(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return rc;
 
   if (rc && rc < (int)sizeof(info)) {
-    CLS_ERR("bad rbd_info object, read %d bytes, expected %d", rc, sizeof(info));
+    CLS_ERR("bad rbd_info object, read %d bytes, expected %d", rc,
+           (int)sizeof(info));
     return -EIO;
   }
 

diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc
index ae9e19b84bcffcfba86a81aa913225c92b339f67..b942ff7d29d4fd85c183dbc22653d54585baa0e3 100644 (file)
--- a/src/cls/rgw/cls_rgw.cc
+++ b/src/cls/rgw/cls_rgw.cc
@@ -326,7 +326,7 @@ static int read_index_entry(cls_method_context_t hctx, string& name, struct rgw_
     return -EIO;
   }
 
-  CLS_LOG(1, "read_index_entry(): existing entry: epoch=%lld name=%s locator=%s\n", entry->epoch, entry->name.c_str(), entry->locator.c_str());
+  CLS_LOG(1, "read_index_entry(): existing entry: epoch=%llu name=%s locator=%s\n", (unsigned long long)entry->epoch, entry->name.c_str(), entry->locator.c_str());
   return 0;
 }
 
@@ -341,7 +341,7 @@ int rgw_bucket_complete_op(cls_method_context_t hctx, bufferlist *in, bufferlist
     CLS_LOG(1, "ERROR: rgw_bucket_complete_op(): failed to decode request\n");
     return -EINVAL;
   }
-  CLS_LOG(1, "rgw_bucket_complete_op(): request: op=%d name=%s epoch=%lld tag=%s\n", op.op, op.name.c_str(), op.epoch, op.tag.c_str());
+  CLS_LOG(1, "rgw_bucket_complete_op(): request: op=%d name=%s epoch=%llu tag=%s\n", op.op, op.name.c_str(), (unsigned long long)op.epoch, op.tag.c_str());
 
   bufferlist header_bl;
   struct rgw_bucket_dir_header header;
@@ -445,7 +445,7 @@ int rgw_bucket_complete_op(cls_method_context_t hctx, bufferlist *in, bufferlist
   }
 
   list<string>::iterator remove_iter;
-  CLS_LOG(0, "rgw_bucket_complete_op(): remove_objs.size()=%d\n", op.remove_objs.size());
+  CLS_LOG(0, "rgw_bucket_complete_op(): remove_objs.size()=%d\n", (int)op.remove_objs.size());
   for (remove_iter = op.remove_objs.begin(); remove_iter != op.remove_objs.end(); ++remove_iter) {
     string& remove_oid_name = *remove_iter;
     CLS_LOG(1, "rgw_bucket_complete_op(): removing entries, read_index_entry name=%s\n", remove_oid_name.c_str());
@@ -534,7 +534,7 @@ int rgw_dir_suggest_changes(cls_method_context_t hctx, bufferlist *in, bufferlis
 
     CLS_LOG(20, "cur_disk.pending_map.empty()=%d op=%d cur_disk.exists=%d cur_change.pending_map.size()=%d cur_change.exists=%d\n",
            cur_disk.pending_map.empty(), (int)op, cur_disk.exists,
-           cur_change.pending_map.size(), cur_change.exists);
+           (int)cur_change.pending_map.size(), cur_change.exists);
 
     if (cur_disk.pending_map.empty()) {
       if (cur_disk.exists) {
@@ -902,7 +902,7 @@ static int gc_omap_remove(cls_method_context_t hctx, int type, const string& key
 static void get_time_key(utime_t& ut, string *key)
 {
   char buf[32];
-  snprintf(buf, 32, "%011lld.%09d", (long long)ut.sec(), ut.nsec());
+  snprintf(buf, 32, "%011llu.%09u", (unsigned long long)ut.sec(), ut.nsec());
   *key = buf;
 }
 

diff --git a/src/key_value_store/cls_kvs.cc b/src/key_value_store/cls_kvs.cc
index 6490cdba98ae15ad1554df986e87331108a3c438..fad46f0ca607eb419423bafa53873ecdbc6e997b 100644 (file)
--- a/src/key_value_store/cls_kvs.cc
+++ b/src/key_value_store/cls_kvs.cc
@@ -218,19 +218,19 @@ static int read_many(cls_method_context_t hctx, const set<string> &keys,
     map<string, bufferlist> * out) {
   int r = 0;
   CLS_ERR("reading from a map of size %d, first key encoded is %s",
-      keys.size(), key_data(*keys.begin()).encoded().c_str());
+      (int)keys.size(), key_data(*keys.begin()).encoded().c_str());
   r = cls_cxx_map_get_vals(hctx, key_data(*keys.begin()).encoded().c_str(),
       "", LONG_MAX, out);
   if (r < 0) {
     CLS_ERR("getting omap vals failed with error %d", r);
   }
 
-  CLS_ERR("got map of size %d ", out->size());
+  CLS_ERR("got map of size %d ", (int)out->size());
   if (out->size() > 1) {
     out->erase(out->upper_bound(key_data(*keys.rbegin()).encoded().c_str()),
       out->end());
   }
-  CLS_ERR("returning map of size %d", out->size());
+  CLS_ERR("returning map of size %d", (int)out->size());
   return r;
 }
 
@@ -315,7 +315,8 @@ static int assert_size_in_bound(cls_method_context_t hctx, int bound,
     }
     break;
   default:
-    CLS_LOG(20, "invalid argument passed to assert_size_in_bound", r);
+    CLS_LOG(20, "invalid argument passed to assert_size_in_bound: %d",
+           comparator);
     return -EINVAL;
   }
   return 0;
@@ -356,7 +357,7 @@ static int omap_insert(cls_method_context_t hctx,
   CLS_LOG(20, "inserting %s", omap.begin()->first.c_str());
   r = check_writable(hctx);
   if (r < 0) {
-    CLS_LOG(20, "omap_insert: this object is unwritable.", r);
+    CLS_LOG(20, "omap_insert: this object is unwritable: %d", r);
     return r;
   }
 
@@ -438,7 +439,7 @@ static int create_with_omap(cls_method_context_t hctx,
   //first make sure the object is writable
   int r = cls_cxx_create(hctx, true);
   if (r < 0) {
-    CLS_LOG(20, "omap create: creating failed: ", r);
+    CLS_LOG(20, "omap create: creating failed: %d", r);
     return r;
   }
 
@@ -621,8 +622,9 @@ static int maybe_read_for_balance(cls_method_context_t hctx,
     return r;
   }
 
-  CLS_LOG(20, "rebalance read: size xattr is %d, omap size is %d", odata.size,
-      odata.omap.size());
+  CLS_LOG(20, "rebalance read: size xattr is %llu, omap size is %llu",
+         (unsigned long long)odata.size,
+         (unsigned long long)odata.omap.size());
   return 0;
 }
 

diff --git a/src/objclass/objclass.h b/src/objclass/objclass.h
index 9f61bcbb5be5ab34ca86f65a1c731c1d04685d85..a9ecd2e9d98e69e3fe52edcb733d35b2208719a0 100644 (file)
--- a/src/objclass/objclass.h
+++ b/src/objclass/objclass.h
@@ -44,7 +44,8 @@ typedef struct {
 } cls_deps_t;
 
 /* class utils */
-extern int cls_log(int level, const char *format, ...);
+extern int cls_log(int level, const char *format, ...)
+  __attribute__((__format__(printf, 2, 3)));
 extern void *cls_alloc(size_t size);
 extern void cls_free(void *p);