headers: Make ceph_le member private

This prevents accidental construction of any ceph_le16/32/64 object
and thereby forces use of the init_le16/32/64 routines so that the
initial value is always byte-swapped as necessary.

This requires a number of follow-on changes where code was accessing
the now private member:

- The associated operator== now needs to be a "friend".

- This code now warns due to overwriting a private member via memcpy:
    copy_from_legacy_head(struct ceph_mds_request_head *head,
              struct ceph_mds_request_head_legacy *legacy) {
      memcpy(&(head->oldest_client_tid), legacy, sizeof(*legacy));
  I'm simply using a struct assignment instead.

In addition, because of the private member the ceph_le types no
longer count as "aggregates", which causes a few issues:

- GCC no longer allows (without warning) the case where a packed struct
  contains *non-packed* structs that contain ceph_le. In practice,
  all those structs and substructs were always intended to be completely
  packed (and actually are, those substructs where the attribute was
  missing happen to be naturally packed), so I've simply added the
  missing attributes.

- Similarly, GCC no longer allows a std::array to be an element of a
  packed struct that also contains a non-aggregate type (like the new
  ceph_le). This happens in one file, msg/async/frames_v2.h. I've
  replaced the std::array with a plain C array with the same layout.

Those changes are all no-ops on all platforms.

Signed-off-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>

diff --git a/src/include/byteorder.h b/src/include/byteorder.h
index 211b0a9441ec897476ac686a1c9a0cbd7ca4de49..03e30f84fc7d6eb57475ffc1d5901f53320242c3 100644 (file)
--- a/src/include/byteorder.h
+++ b/src/include/byteorder.h
@@ -66,19 +66,19 @@ inline T mswab(T val) {
 
 template<typename T>
 struct ceph_le {
+private:
   T v;
+public:
   ceph_le<T>& operator=(T nv) {
     v = mswab(nv);
     return *this;
   }
   operator T() const { return mswab(v); }
+  friend inline bool operator==(ceph_le a, ceph_le b) {
+    return a.v == b.v;
+  }
 } __attribute__ ((packed));
 
-template<typename T>
-inline bool operator==(ceph_le<T> a, ceph_le<T> b) {
-  return a.v == b.v;
-}
-
 using ceph_le64 = ceph_le<__u64>;
 using ceph_le32 = ceph_le<__u32>;
 using ceph_le16 = ceph_le<__u16>;

diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h
index 3b034939ee7d81d369316e87562952c9bd20c01a..d5b56e0df80d3794364b51c5c0fd72315dad7131 100644 (file)
--- a/src/include/ceph_fs.h
+++ b/src/include/ceph_fs.h
@@ -637,14 +637,18 @@ static inline void
 copy_from_legacy_head(struct ceph_mds_request_head *head,
                        struct ceph_mds_request_head_legacy *legacy)
 {
-       memcpy(&(head->oldest_client_tid), legacy, sizeof(*legacy));
+       struct ceph_mds_request_head_legacy *embedded_legacy =
+               (struct ceph_mds_request_head_legacy *)&head->oldest_client_tid;
+       *embedded_legacy = *legacy;
 }
 
 static inline void
 copy_to_legacy_head(struct ceph_mds_request_head_legacy *legacy,
                        struct ceph_mds_request_head *head)
 {
-       memcpy(legacy, &(head->oldest_client_tid), sizeof(*legacy));
+       struct ceph_mds_request_head_legacy *embedded_legacy =
+               (struct ceph_mds_request_head_legacy *)&head->oldest_client_tid;
+       *legacy = *embedded_legacy;
 }
 
 /* client reply */
@@ -875,10 +879,10 @@ struct ceph_mds_caps_body_legacy {
                        struct ceph_timespec mtime, atime, ctime;
                        struct ceph_file_layout layout;
                        __le32 time_warp_seq;
-               };
+               } __attribute__ ((packed));
                /* export message */
                struct ceph_mds_cap_peer peer;
-       };
+       } __attribute__ ((packed));
 } __attribute__ ((packed));
 
 /* cap release msg head */

diff --git a/src/include/rados.h b/src/include/rados.h
index 8e35994daed2a986cf59b639a5453cf20f39b0d1..1a6f2ec140d3144beb0e50b14800059b523c68c3 100644 (file)
--- a/src/include/rados.h
+++ b/src/include/rados.h
@@ -641,7 +641,7 @@ struct ceph_osd_op {
                        __le32 chunk_size;
                        __u8 type;              /* CEPH_OSD_CHECKSUM_OP_TYPE_* */
                } __attribute__ ((packed)) checksum;
-       };
+       } __attribute__ ((packed));
        __le32 payload_len;
 } __attribute__ ((packed));
 

diff --git a/src/msg/async/frames_v2.h b/src/msg/async/frames_v2.h
index de28772e33cc29fb057cbf8b46fa127cbf7615d8..3f8708f237af6716bbaa56d9fe7e15a2ae9a19a6 100644 (file)
--- a/src/msg/async/frames_v2.h
+++ b/src/msg/async/frames_v2.h
@@ -98,7 +98,7 @@ struct preamble_block_t {
   // third to #segments - MAX_NUM_SEGMENTS and so on.
   __u8 num_segments;
 
-  std::array<segment_t, MAX_NUM_SEGMENTS> segments;
+  segment_t segments[MAX_NUM_SEGMENTS];
   __u8 _reserved[2];
 
   // CRC32 for this single preamble block.
@@ -128,7 +128,7 @@ static_assert(std::is_standard_layout<preamble_block_t>::value);
 // frame abortion facility.
 struct epilogue_plain_block_t {
   __u8 late_flags;
-  std::array<ceph_le32, MAX_NUM_SEGMENTS> crc_values;
+  ceph_le32 crc_values[MAX_NUM_SEGMENTS];
 } __attribute__((packed));
 static_assert(std::is_standard_layout<epilogue_plain_block_t>::value);
 
@@ -177,8 +177,7 @@ private:
   };
   ceph::bufferlist::contiguous_filler preamble_filler;
 
-  __u8 calc_num_segments(
-    const std::array<segment_t, MAX_NUM_SEGMENTS>& segments)
+  __u8 calc_num_segments(const segment_t segments[])
   {
     for (__u8 num = SegmentsNumV; num > 0; num--) {
       if (segments[num-1].length) {
@@ -203,9 +202,9 @@ private:
     // implementation detail: the first bufferlist of Frame::segments carries
     // space for preamble. This glueing isn't a part of the onwire format but
     // just our private detail.
-    main_preamble.segments.front().length =
-        segments.front().length() - FRAME_PREAMBLE_SIZE;
-    main_preamble.segments.front().alignment = alignments.front();
+    main_preamble.segments[0].length =
+        segments[0].length() - FRAME_PREAMBLE_SIZE;
+    main_preamble.segments[0].alignment = alignments[0];
 
     // there is no business in issuing frame without at least one segment
     // filled.

diff --git a/src/os/Transaction.h b/src/os/Transaction.h
index 1463d006159168ebb30053c05f85a9d9418d04cc..f74f19d2c8b7a5e2f6328594fce7408f42c2650d 100644 (file)
--- a/src/os/Transaction.h
+++ b/src/os/Transaction.h
@@ -170,11 +170,11 @@ public:
     union {
        struct {
          ceph_le32 hint_type;          //OP_COLL_HINT
-       };
+       } __attribute__ ((packed));
        struct {
          ceph_le32 alloc_hint_flags;   //OP_SETALLOCHINT
-       };
-    };
+       } __attribute__ ((packed));
+    } __attribute__ ((packed));
     ceph_le64 expected_object_size;   //OP_SETALLOCHINT
     ceph_le64 expected_write_size;    //OP_SETALLOCHINT
     ceph_le32 split_bits;             //OP_SPLIT_COLLECTION2,OP_COLL_SET_BITS,