Skip to content

Commit 6e4da22

Browse files
ronen-frronen-fr
authored
Merge pull request ceph#61766 from aainscow/shard_id_t
osd: Require explicit casting for shard_id_t Reviewed-by: Bill Scales <[email protected]> Reviewed-by: Ronen Friedman <[email protected]>
2 parents 3040a6d + 0d0f38f commit 6e4da22

File tree

24 files changed

+105
-72
lines changed

24 files changed

+105
-72
lines changed

‎src/common/hobject.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ void ghobject_t::dump(Formatter *f) const
491491
if (generation != NO_GEN)
492492
f->dump_int("generation", generation);
493493
if (shard_id != shard_id_t::NO_SHARD)
494-
f->dump_int("shard_id", shard_id);
494+
f->dump_int("shard_id", int(shard_id));
495495
f->dump_int("max", (int)max);
496496
}
497497

@@ -548,14 +548,16 @@ bool ghobject_t::parse(const string& s)
548548
// look for shard# prefix
549549
const char *start = s.c_str();
550550
const char *p;
551-
int sh = shard_id_t::NO_SHARD;
551+
shard_id_t sh = shard_id_t::NO_SHARD;
552552
for (p = start; *p && isxdigit(*p); ++p) ;
553553
if (!*p && *p != '#')
554554
return false;
555555
if (p > start) {
556-
int r = sscanf(s.c_str(), "%x", &sh);
556+
unsigned int sh_i;
557+
int r = sscanf(s.c_str(), "%x", &sh_i);
557558
if (r < 1)
558559
return false;
560+
sh = shard_id_t(sh_i);
559561
start = p + 1;
560562
} else {
561563
++start;

‎src/common/scrub_types.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ inconsistent_obj_wrapper::set_auth_missing(const hobject_t& hoid,
139139
else if (shard_map[pg_map.first].has_shallow_errors())
140140
++shallow_errors;
141141
union_shards.errors |= shard_map[pg_map.first].errors;
142-
shards.emplace(osd_shard_t{pg_map.first.osd, pg_map.first.shard}, shard_map[pg_map.first]);
142+
shards.emplace(osd_shard_t{pg_map.first.osd,
143+
static_cast<int8_t>(pg_map.first.shard)}, shard_map[pg_map.first]);
143144
}
144145
}
145146

‎src/crimson/admin/osd_admin.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ static ghobject_t test_ops_get_object_name(
465465

466466
auto shard_id = cmd_getval_or<int64_t>(cmdmap,
467467
"shardid",
468-
shard_id_t::NO_SHARD);
468+
static_cast<int64_t>(shard_id_t::NO_SHARD));
469469

470470
return ghobject_t{
471471
hobject_t{

‎src/crimson/os/seastore/onode_manager/staged-fltree/stages/key_layout.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ class key_hobj_t {
430430
* common interfaces as a full_key_t
431431
*/
432432
shard_t shard() const {
433-
return ghobj.shard_id;
433+
return static_cast<shard_t>(ghobj.shard_id);
434434
}
435435
pool_t pool() const {
436436
return ghobj.hobj.pool;

‎src/crimson/osd/scrub/scrub_validator.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ object_evaluation_t evaluate_object(
306306
[](auto &cand) { return cand.has_errors(); })) {
307307
for (auto &eval : shards) {
308308
iow.shards.emplace(
309-
librados::osd_shard_t{eval.source.osd, eval.source.shard},
309+
librados::osd_shard_t{eval.source.osd, static_cast<int8_t>(eval.source.shard)},
310310
eval.shard_info);
311311
iow.union_shards.errors |= eval.shard_info.errors;
312312
}

‎src/include/types.h‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,12 @@ struct shard_id_t {
521521
int8_t id;
522522

523523
shard_id_t() : id(0) {}
524-
constexpr explicit shard_id_t(int8_t _id) : id(_id) {}
524+
explicit constexpr shard_id_t(int8_t _id) : id(_id) {}
525525

526-
constexpr operator int8_t() const { return id; }
526+
explicit constexpr operator int8_t() const { return id; }
527+
explicit constexpr operator int64_t() const { return id; }
528+
explicit constexpr operator int() const { return id; }
529+
explicit constexpr operator unsigned() const { return id; }
527530

528531
const static shard_id_t NO_SHARD;
529532

@@ -542,8 +545,26 @@ struct shard_id_t {
542545
ls.push_back(new shard_id_t(1));
543546
ls.push_back(new shard_id_t(2));
544547
}
545-
bool operator==(const shard_id_t&) const = default;
546-
auto operator<=>(const shard_id_t&) const = default;
548+
shard_id_t& operator++() { ++id; return *this; }
549+
friend constexpr std::strong_ordering operator<=>(const shard_id_t &lhs,
550+
const shard_id_t &rhs) {
551+
return lhs.id <=> rhs.id;
552+
}
553+
554+
friend constexpr std::strong_ordering operator<=>(int lhs,
555+
const shard_id_t &rhs) {
556+
return lhs <=> rhs.id;
557+
}
558+
friend constexpr std::strong_ordering operator<=>(const shard_id_t &lhs,
559+
int rhs) {
560+
return lhs.id <=> rhs;
561+
}
562+
563+
shard_id_t& operator=(int other) { id = other; return *this; }
564+
bool operator==(const shard_id_t &other) const { return id == other.id; }
565+
566+
shard_id_t operator+(int other) const { return shard_id_t(id + other); }
567+
shard_id_t operator-(int other) const { return shard_id_t(id - other); }
547568
};
548569
WRITE_CLASS_ENCODER(shard_id_t)
549570
std::ostream &operator<<(std::ostream &lhs, const shard_id_t &rhs);

‎src/osd/ECBackend.cc‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,13 @@ void ECBackend::RecoveryBackend::handle_recovery_read_complete(
374374
for (set<shard_id_t>::iterator i = op.missing_on_shards.begin();
375375
i != op.missing_on_shards.end();
376376
++i) {
377-
target[*i] = &(op.returned_data[*i]);
377+
target[static_cast<int>(*i)] = &(op.returned_data[static_cast<int>(*i)]);
378378
}
379379
map<int, bufferlist> from;
380380
for(map<pg_shard_t, bufferlist>::iterator i = to_read.get<2>().begin();
381381
i != to_read.get<2>().end();
382382
++i) {
383-
from[i->first.shard] = std::move(i->second);
383+
from[static_cast<int>(i->first.shard)] = std::move(i->second);
384384
}
385385
dout(10) << __func__ << ": " << from << dendl;
386386
int r;
@@ -635,12 +635,12 @@ void ECBackend::RecoveryBackend::continue_recovery_op(
635635
for (set<pg_shard_t>::iterator mi = op.missing_on.begin();
636636
mi != op.missing_on.end();
637637
++mi) {
638-
ceph_assert(op.returned_data.count(mi->shard));
638+
ceph_assert(op.returned_data.count(static_cast<int>(mi->shard)));
639639
m->pushes[*mi].push_back(PushOp());
640640
PushOp &pop = m->pushes[*mi].back();
641641
pop.soid = op.hoid;
642642
pop.version = op.v;
643-
pop.data = op.returned_data[mi->shard];
643+
pop.data = op.returned_data[static_cast<int>(mi->shard)];
644644
dout(10) << __func__ << ": before_progress=" << op.recovery_progress
645645
<< ", after_progress=" << after_progress
646646
<< ", pop.data.length()=" << pop.data.length()
@@ -1124,11 +1124,11 @@ void ECBackend::handle_sub_read(
11241124
dout(20) << __func__ << ": Checking hash of " << i->first << dendl;
11251125
bufferhash h(-1);
11261126
h << bl;
1127-
if (h.digest() != hinfo->get_chunk_hash(shard)) {
1127+
if (h.digest() != hinfo->get_chunk_hash(static_cast<int>(shard))) {
11281128
get_parent()->clog_error() << "Bad hash for " << i->first << " digest 0x"
1129-
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(shard) << dec;
1129+
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(static_cast<int>(shard)) << dec;
11301130
dout(5) << __func__ << ": Bad hash for " << i->first << " digest 0x"
1131-
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(shard) << dec << dendl;
1131+
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(static_cast<int>(shard)) << dec << dendl;
11321132
r = -EIO;
11331133
goto error;
11341134
}
@@ -1305,7 +1305,7 @@ void ECBackend::handle_sub_read_reply(
13051305
iter->second.returned.front().get<2>().begin();
13061306
j != iter->second.returned.front().get<2>().end();
13071307
++j) {
1308-
have.insert(j->first.shard);
1308+
have.insert(static_cast<int>(j->first.shard));
13091309
dout(20) << __func__ << " have shard=" << j->first.shard << dendl;
13101310
}
13111311
map<int, vector<pair<int, int>>> dummy_minimum;
@@ -1800,11 +1800,11 @@ int ECBackend::be_deep_scrub(
18001800
return 0;
18011801
}
18021802

1803-
if (hinfo->get_chunk_hash(get_parent()->whoami_shard().shard) !=
1803+
if (hinfo->get_chunk_hash(static_cast<int>(get_parent()->whoami_shard().shard)) !=
18041804
pos.data_hash.digest()) {
18051805
dout(0) << "_scan_list " << poid << " got incorrect hash on read 0x"
18061806
<< std::hex << pos.data_hash.digest() << " != expected 0x"
1807-
<< hinfo->get_chunk_hash(get_parent()->whoami_shard().shard)
1807+
<< hinfo->get_chunk_hash(static_cast<int>(get_parent()->whoami_shard().shard))
18081808
<< std::dec << dendl;
18091809
o.ec_hash_mismatch = true;
18101810
return 0;

‎src/osd/ECBackend.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ class ECBackend : public ECCommon {
366366
for (std::set<pg_shard_t>::const_iterator i = _have.begin();
367367
i != _have.end();
368368
++i) {
369-
have.insert(i->shard);
369+
have.insert(static_cast<int>(i->shard));
370370
}
371371
std::map<int, std::vector<std::pair<int, int>>> min;
372372
return ec_impl->minimum_to_decode(want, have, &min) == 0;

‎src/osd/ECBackendL.cc‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -376,13 +376,15 @@ void ECBackendL::RecoveryBackend::handle_recovery_read_complete(
376376
for (set<shard_id_t>::iterator i = op.missing_on_shards.begin();
377377
i != op.missing_on_shards.end();
378378
++i) {
379-
target[*i] = &(op.returned_data[*i]);
379+
int s = static_cast<int>(*i);
380+
target[s] = &(op.returned_data[s]);
380381
}
381382
map<int, bufferlist> from;
382383
for(map<pg_shard_t, bufferlist>::iterator i = to_read.get<2>().begin();
383384
i != to_read.get<2>().end();
384385
++i) {
385-
from[i->first.shard] = std::move(i->second);
386+
int s = static_cast<int>(i->first.shard);
387+
from[s] = std::move(i->second);
386388
}
387389
dout(10) << __func__ << ": " << from << dendl;
388390
int r;
@@ -637,12 +639,12 @@ void ECBackendL::RecoveryBackend::continue_recovery_op(
637639
for (set<pg_shard_t>::iterator mi = op.missing_on.begin();
638640
mi != op.missing_on.end();
639641
++mi) {
640-
ceph_assert(op.returned_data.count(mi->shard));
642+
ceph_assert(op.returned_data.count(static_cast<int>(mi->shard)));
641643
m->pushes[*mi].push_back(PushOp());
642644
PushOp &pop = m->pushes[*mi].back();
643645
pop.soid = op.hoid;
644646
pop.version = op.v;
645-
pop.data = op.returned_data[mi->shard];
647+
pop.data = op.returned_data[static_cast<int>(mi->shard)];
646648
dout(10) << __func__ << ": before_progress=" << op.recovery_progress
647649
<< ", after_progress=" << after_progress
648650
<< ", pop.data.length()=" << pop.data.length()
@@ -1126,11 +1128,12 @@ void ECBackendL::handle_sub_read(
11261128
dout(20) << __func__ << ": Checking hash of " << i->first << dendl;
11271129
bufferhash h(-1);
11281130
h << bl;
1129-
if (h.digest() != hinfo->get_chunk_hash(shard)) {
1131+
int s = static_cast<int>(shard);
1132+
if (h.digest() != hinfo->get_chunk_hash(s)) {
11301133
get_parent()->clog_error() << "Bad hash for " << i->first << " digest 0x"
1131-
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(shard) << dec;
1134+
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(s) << dec;
11321135
dout(5) << __func__ << ": Bad hash for " << i->first << " digest 0x"
1133-
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(shard) << dec << dendl;
1136+
<< hex << h.digest() << " expected 0x" << hinfo->get_chunk_hash(s) << dec << dendl;
11341137
r = -EIO;
11351138
goto error;
11361139
}
@@ -1307,7 +1310,7 @@ void ECBackendL::handle_sub_read_reply(
13071310
iter->second.returned.front().get<2>().begin();
13081311
j != iter->second.returned.front().get<2>().end();
13091312
++j) {
1310-
have.insert(j->first.shard);
1313+
have.insert(static_cast<int>(j->first.shard));
13111314
dout(20) << __func__ << " have shard=" << j->first.shard << dendl;
13121315
}
13131316
map<int, vector<pair<int, int>>> dummy_minimum;
@@ -1802,11 +1805,11 @@ int ECBackendL::be_deep_scrub(
18021805
return 0;
18031806
}
18041807

1805-
if (hinfo->get_chunk_hash(get_parent()->whoami_shard().shard) !=
1806-
pos.data_hash.digest()) {
1808+
int s = static_cast<int>(get_parent()->whoami_shard().shard);
1809+
if (hinfo->get_chunk_hash(s) != pos.data_hash.digest()) {
18071810
dout(0) << "_scan_list " << poid << " got incorrect hash on read 0x"
18081811
<< std::hex << pos.data_hash.digest() << " != expected 0x"
1809-
<< hinfo->get_chunk_hash(get_parent()->whoami_shard().shard)
1812+
<< hinfo->get_chunk_hash(s)
18101813
<< std::dec << dendl;
18111814
o.ec_hash_mismatch = true;
18121815
return 0;

‎src/osd/ECBackendL.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ friend struct RecoveryReadCompleter;
366366
for (std::set<pg_shard_t>::const_iterator i = _have.begin();
367367
i != _have.end();
368368
++i) {
369-
have.insert(i->shard);
369+
have.insert(static_cast<int>(i->shard));
370370
}
371371
std::map<int, std::vector<std::pair<int, int>>> min;
372372
return ec_impl->minimum_to_decode(want, have, &min) == 0;

0 commit comments

Comments
 (0)