Skip to content

Commit 1ec3767

Browse files
committed
correct issues with id remapping in apply ordering
We had the possibility of out-of-bounds access to the id remapping array, leading to segfaults and other fun.
1 parent 8959a5c commit 1ec3767

File tree

2 files changed

+8
-18
lines changed

2 files changed

+8
-18
lines changed

src/node.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,9 @@ void node_t::apply_ordering(
354354
uint64_t j = 0;
355355
for (uint64_t i = 0; i < decoding.size(); ++i) {
356356
uint64_t old_id = decode(i);
357-
if (old_id) {
358-
dec_v.push_back(get_new_id(old_id));
357+
uint64_t new_id = old_id ? get_new_id(old_id) : 0;
358+
if (new_id) {
359+
dec_v.push_back(new_id);
359360
encoding_map.push_back(j++);
360361
} else {
361362
// this means that the node referred to by this entry has been deleted

src/odgi.cpp

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -762,30 +762,19 @@ void graph_t::apply_ordering(const std::vector<handle_t>& order_in, bool compact
762762
}
763763

764764
// establish id mapping
765-
uint64_t max_handle_rank = 0;
766-
uint64_t min_handle_rank = std::numeric_limits<uint64_t>::max();
767-
{
768-
uint64_t tmp;
769-
for_each_handle([&](const handle_t& handle) {
770-
tmp = number_bool_packing::unpack_number(handle);
771-
max_handle_rank = std::max(max_handle_rank, tmp);
772-
min_handle_rank = std::min(min_handle_rank, tmp);
773-
});
774-
}
775-
776765
std::vector<std::pair<nid_t, bool>> ids;
777-
// fill even for deleted nodes
766+
// fill even for deleted nodes, which we map to 0
778767
ids.resize(node_v.size(), std::make_pair(0, false));
779768

780769
if (compact_ids) {
781770
for (uint64_t i = 0; i < order->size(); ++i) {
782-
ids[number_bool_packing::unpack_number(order->at(i)) - min_handle_rank] =
771+
ids[number_bool_packing::unpack_number(order->at(i))] =
783772
std::make_pair(i+1,
784773
get_is_reverse(order->at(i)));
785774
}
786775
} else {
787776
for (auto handle : *order) {
788-
ids[number_bool_packing::unpack_number(handle) - min_handle_rank] =
777+
ids[number_bool_packing::unpack_number(handle)] =
789778
std::make_pair(get_id(handle),
790779
get_is_reverse(handle));
791780
}
@@ -794,11 +783,11 @@ void graph_t::apply_ordering(const std::vector<handle_t>& order_in, bool compact
794783
// helpers to map from current to new id and orientation
795784
auto get_new_id =
796785
[&](uint64_t id) {
797-
return ids[id - 1 - min_handle_rank].first;
786+
return ids[id - 1].first;
798787
};
799788
auto to_flip =
800789
[&](uint64_t id) {
801-
return ids[id - 1 - min_handle_rank].second;
790+
return ids[id - 1].second;
802791
};
803792

804793
// nodes, edges, and path steps

0 commit comments

Comments
 (0)