include/intarith: enforce the same type for p2*() arguments

x and align must be of the same width.  Otherwise the lack of
sign-extension can bite, e.g.:

  uint64_t x = 128000;
  uint32_t align = 32768;
  uint64_t res = p2roundup(x, align);
  // res = 18446744069414715392

While the templates added in commit c06b97b3d7e3 ("include: Add
templates along side macros in intarith") are technically correct,
P2*() macros weren't meant to be used with different types.  The
Solaris kernel (which is where they originated) has P2*_TYPED()
variants for that.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/src/auth/Crypto.cc b/src/auth/Crypto.cc
index 2143a0699765b21fe4bc89a3adab601dd8340864..bf9270a92e2cbb0364640e69914365d7e4933243 100644 (file)
--- a/src/auth/Crypto.cc
+++ b/src/auth/Crypto.cc
@@ -234,7 +234,7 @@ public:
     //   16 + p2align(10, 16) -> 16
     //   16 + p2align(16, 16) -> 32 including 16 bytes for padding.
     ceph::bufferptr out_tmp{static_cast<unsigned>(
-      AES_BLOCK_LEN + p2align(in.length(), AES_BLOCK_LEN))};
+      AES_BLOCK_LEN + p2align<std::size_t>(in.length(), AES_BLOCK_LEN))};
 
     // let's pad the data
     std::uint8_t pad_len = out_tmp.length() - in.length();
@@ -301,9 +301,7 @@ public:
     if (out.buf == nullptr) {
       // 16 + p2align(10, 16) -> 16
       // 16 + p2align(16, 16) -> 32
-      const std::size_t needed = \
-        AES_BLOCK_LEN + p2align(in.length, AES_BLOCK_LEN);
-      return needed;
+      return AES_BLOCK_LEN + p2align<std::size_t>(in.length, AES_BLOCK_LEN);
     }
 
     // how many bytes of in.buf hang outside the alignment boundary and how

diff --git a/src/include/intarith.h b/src/include/intarith.h
index 242056e216f2e08a2183851ea41cc12205ed596e..e912cbe7b3c44f242921f298dcd8dd0b4dc11e16 100644 (file)
--- a/src/include/intarith.h
+++ b/src/include/intarith.h
@@ -53,8 +53,8 @@ constexpr inline bool isp2(T x) {
  * eg, p2align(0x1234, 0x100) == 0x1200 (0x12*align)
  * eg, p2align(0x5600, 0x100) == 0x5600 (0x56*align)
  */
-template<typename T, typename U>
-constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2align(T x, U align) {
+template<typename T>
+constexpr inline T p2align(T x, T align) {
   return x & -align;
 }
 
@@ -63,8 +63,8 @@ constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2align(T x, U a
  * eg, p2phase(0x1234, 0x100) == 0x34 (x-0x12*align)
  * eg, p2phase(0x5600, 0x100) == 0x00 (x-0x56*align)
  */
-template<typename T, typename U>
-constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2phase(T x, U align) {
+template<typename T>
+constexpr inline T p2phase(T x, T align) {
   return x & (align - 1);
 }
 
@@ -74,8 +74,8 @@ constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2phase(T x, U a
  * eg, p2nphase(0x1234, 0x100) == 0xcc (0x13*align-x)
  * eg, p2nphase(0x5600, 0x100) == 0x00 (0x56*align-x)
  */
-template<typename T, typename U>
-constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2nphase(T x, U align) {
+template<typename T>
+constexpr inline T p2nphase(T x, T align) {
   return -x & (align - 1);
 }
 
@@ -84,9 +84,9 @@ constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2nphase(T x, U
  * eg, p2roundup(0x1234, 0x100) == 0x1300 (0x13*align)
  * eg, p2roundup(0x5600, 0x100) == 0x5600 (0x56*align)
  */
-template<typename T, typename U>
-constexpr inline std::make_unsigned_t<std::common_type_t<T, U>> p2roundup(T x, U align) {
-  return (-(-(x) & -(align)));
+template<typename T>
+constexpr inline T p2roundup(T x, T align) {
+  return -(-x & -align);
 }
 
 // count trailing zeros.

diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc
index 1b6d9756d4af04a5404f8b3c775f3ac9e39679ac..a86720621c6b1eb7789f1aa8bd46cf7f2240b208 100644 (file)
--- a/src/librbd/io/ImageRequest.cc
+++ b/src/librbd/io/ImageRequest.cc
@@ -556,8 +556,8 @@ int ImageDiscardRequest<I>::prune_object_extents(
   // is a special case for filestore
   bool prune_required = false;
   auto object_size = this->m_image_ctx.layout.object_size;
-  auto discard_granularity_bytes = std::min<uint64_t>(
-    m_discard_granularity_bytes, object_size);
+  auto discard_granularity_bytes = std::min(m_discard_granularity_bytes,
+                                            object_size);
   auto xform_lambda =
     [discard_granularity_bytes, object_size, &prune_required]
     (ObjectExtent& object_extent) {
@@ -567,9 +567,8 @@ int ImageDiscardRequest<I>::prune_object_extents(
 
       if ((discard_granularity_bytes < object_size) ||
           (next_offset < object_size)) {
-        static_assert(sizeof(offset) == sizeof(discard_granularity_bytes));
-        offset = p2roundup(offset, discard_granularity_bytes);
-        next_offset = p2align(next_offset, discard_granularity_bytes);
+        offset = p2roundup<uint64_t>(offset, discard_granularity_bytes);
+        next_offset = p2align<uint64_t>(next_offset, discard_granularity_bytes);
         if (offset >= next_offset) {
           prune_required = true;
           length = 0;

diff --git a/src/msg/async/frames_v2.h b/src/msg/async/frames_v2.h
index 7093e244a2657bd45a0e41585f32920255525b85..b99269702a53fad211a6108125f3010b39c25fdf 100644 (file)
--- a/src/msg/async/frames_v2.h
+++ b/src/msg/async/frames_v2.h
@@ -150,7 +150,7 @@ static constexpr uint32_t FRAME_SECURE_EPILOGUE_SIZE =
 
 static uint32_t segment_onwire_size(const uint32_t logical_size)
 {
-  return p2roundup(logical_size, CRYPTO_BLOCK_SIZE);
+  return p2roundup<uint32_t>(logical_size, CRYPTO_BLOCK_SIZE);
 }
 
 static ceph::bufferlist segment_onwire_bufferlist(ceph::bufferlist&& bl)

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index c9147989c21a4f8b9d23d15cabefc456ecdac5a9..a34e908c0b9e47a3ee4f16e41b6424111d28e9e4 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -11375,7 +11375,7 @@ void BlueStore::_do_write_small(
   auto max_bsize = std::max(wctx->target_blob_size, min_alloc_size);
   auto min_off = offset >= max_bsize ? offset - max_bsize : 0;
   uint32_t alloc_len = min_alloc_size;
-  auto offset0 = p2align(offset, alloc_len);
+  auto offset0 = p2align<uint64_t>(offset, alloc_len);
 
   bool any_change;
 
@@ -11643,7 +11643,7 @@ void BlueStore::_do_write_small(
   // new blob.
   
   BlobRef b = c->new_blob();
-  uint64_t b_off = p2phase(offset, alloc_len);
+  uint64_t b_off = p2phase<uint64_t>(offset, alloc_len);
   uint64_t b_off0 = b_off;
   _pad_zeros(&bl, &b_off0, block_size);
   o->extent_map.punch_hole(c, offset, length, &wctx->old_extents);

diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc
index 411711e285798c99eb271e4c016ac7b55e2d1a42..13f38d692e9fa9838d68c1d66e3b5d78c82a0a1b 100644 (file)
--- a/src/os/bluestore/StupidAllocator.cc
+++ b/src/os/bluestore/StupidAllocator.cc
@@ -256,7 +256,7 @@ double StupidAllocator::get_fragmentation(uint64_t alloc_unit)
   uint64_t intervals = 0;
   {
     std::lock_guard l(lock);
-    max_intervals = p2roundup(num_free, alloc_unit) / alloc_unit;
+    max_intervals = p2roundup<uint64_t>(num_free, alloc_unit) / alloc_unit;
     for (unsigned bin = 0; bin < free.size(); ++bin) {
       intervals += free[bin].num_intervals();
     }

diff --git a/src/os/bluestore/fastbmap_allocator_impl.cc b/src/os/bluestore/fastbmap_allocator_impl.cc
index ccd012b4433baeac6f7dbb6fece9b4939617156a..5919bce45a10b4d3b1750167d57e1e76a6f26313 100755 (executable)
--- a/src/os/bluestore/fastbmap_allocator_impl.cc
+++ b/src/os/bluestore/fastbmap_allocator_impl.cc
@@ -23,7 +23,7 @@ inline interval_t _align2units(uint64_t offset, uint64_t len, uint64_t min_lengt
     auto delta_off = res.offset - offset;
     if (len > delta_off) {
       res.length = len - delta_off;
-      res.length = p2align(res.length, min_length);
+      res.length = p2align<uint32_t>(res.length, min_length);
       if (res.length) {
        return res;
       }
@@ -190,7 +190,7 @@ void AllocatorLevel01Loose::_analyze_partials(uint64_t pos_start,
            (ctx->min_affordable_len == 0 ||
              (longest.length < ctx->min_affordable_len))) {
 
-          ctx->min_affordable_len = p2align(longest.length, min_length);
+          ctx->min_affordable_len = p2align<uint32_t>(longest.length, min_length);
          ctx->min_affordable_offs = longest.offset;
         }
         if (mode == STOP_ON_PARTIAL) {
@@ -288,13 +288,13 @@ void AllocatorLevel01Loose::_mark_alloc_l0(int64_t l0_pos_start,
   int64_t pos = l0_pos_start;
   slot_t bits = (slot_t)1 << (l0_pos_start % d0);
 
-  while (pos < std::min(l0_pos_end, (int64_t)p2roundup(l0_pos_start, d0))) {
+  while (pos < std::min(l0_pos_end, p2roundup<int64_t>(l0_pos_start, d0))) {
     l0[pos / d0] &= ~bits;
     bits <<= 1;
     pos++;
   }
 
-  while (pos < std::min(l0_pos_end, (int64_t)p2align(l0_pos_end, d0))) {
+  while (pos < std::min(l0_pos_end, p2align<int64_t>(l0_pos_end, d0))) {
     l0[pos / d0] = all_slot_clear;
     pos += d0;
   }

diff --git a/src/os/bluestore/fastbmap_allocator_impl.h b/src/os/bluestore/fastbmap_allocator_impl.h
index a27f69f9628808d8aeee175f84373d0b8b8128d5..6b138205a3dff81862db980b2d7bac088ca41cff 100755 (executable)
--- a/src/os/bluestore/fastbmap_allocator_impl.h
+++ b/src/os/bluestore/fastbmap_allocator_impl.h
@@ -336,13 +336,14 @@ protected:
     auto pos = l0_pos_start;
     slot_t bits = (slot_t)1 << (l0_pos_start % d0);
     slot_t& val_s = l0[pos / d0];
-    int64_t pos_e = std::min(l0_pos_end, (int64_t)p2roundup(l0_pos_start + 1, d0));
+    int64_t pos_e = std::min(l0_pos_end,
+                             p2roundup<int64_t>(l0_pos_start + 1, d0));
     while (pos < pos_e) {
       val_s |= bits;
       bits <<= 1;
       pos++;
     }
-    pos_e = std::min(l0_pos_end, (int64_t)p2align(l0_pos_end, d0));
+    pos_e = std::min(l0_pos_end, p2align<int64_t>(l0_pos_end, d0));
     auto idx = pos / d0;
     while (pos < pos_e) {
       l0[idx++] = all_slot_set;