From: Ulrich Weigand Date: Mon, 23 Sep 2019 20:17:36 +0000 (+0200) Subject: headers: Make ceph_le member private X-Git-Tag: v15.1.0~1387^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c70f779d12a2925e72b403e7f3384a83bb9d7978;p=ceph.git 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 --- diff --git a/src/include/byteorder.h b/src/include/byteorder.h index 211b0a9441ec..03e30f84fc7d 100644 --- a/src/include/byteorder.h +++ b/src/include/byteorder.h @@ -66,19 +66,19 @@ inline T mswab(T val) { template struct ceph_le { +private: T v; +public: ceph_le& 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 -inline bool operator==(ceph_le a, ceph_le 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 3b034939ee7d..d5b56e0df80d 100644 --- 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 8e35994daed2..1a6f2ec140d3 100644 --- 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 de28772e33cc..3f8708f237af 100644 --- 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 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::value); // frame abortion facility. struct epilogue_plain_block_t { __u8 late_flags; - std::array crc_values; + ceph_le32 crc_values[MAX_NUM_SEGMENTS]; } __attribute__((packed)); static_assert(std::is_standard_layout::value); @@ -177,8 +177,7 @@ private: }; ceph::bufferlist::contiguous_filler preamble_filler; - __u8 calc_num_segments( - const std::array& 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 1463d0061591..f74f19d2c8b7 100644 --- 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,