Skip to content

Commit 652c180

Browse files
bucket_sorter bug fix in "remove"
removing an element from a stack does not always work, since the predecessor of a stack element is not (always) stored. this patch merges the array head into next, so a top element in a stack can point to the head of the stack it is in. remarks: - i am guessing the intended use (see test below), as it works like that for elements deeper down in a stack - the fix abuses pointers/iterators and infers offsets from address differences. (yes it's a bit ugly.) - memory needs and complexity are unaffected, size_type is probably big enough. test case. B is a bucketsorter operating on a vector V. V[0]=0; V[1]=1; B.push(0); B.push(1); // now, stacks 0 and 1 are singletons. // try to move 0 to stack #1, should result in // head_1->0->1->end, but calls remove first, then V[0]=1; B.update(0); // <- BOOM // the update calls remove, it "removes" 0 from stack #V[0]=1. // it's not there yet (!). // instead 1 (top of bucket #1) must die. // the result is head_1->0->end, and wrong.
1 parent 7913228 commit 652c180

File tree

1 file changed

+14
-15
lines changed

1 file changed

+14
-15
lines changed

include/boost/pending/bucket_sorter.hpp

+14-15
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// Revision History:
1313
// 13 June 2001: Changed some names for clarity. (Jeremy Siek)
1414
// 01 April 2001: Modified to use new <boost/limits.hpp> header. (JMaddock)
15+
// 28 Feb 2017: change bucket head, fix bug in remove. (Felix Salfelder)
1516
//
1617
#ifndef BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
1718
#define BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
@@ -33,9 +34,9 @@ namespace boost {
3334
bucket_sorter(size_type _length, bucket_type _max_bucket,
3435
const Bucket& _bucket = Bucket(),
3536
const ValueIndexMap& _id = ValueIndexMap())
36-
: head(_max_bucket, invalid_value()),
37-
next(_length, invalid_value()),
37+
: next(_length+_max_bucket, invalid_value()),
3838
prev(_length, invalid_value()),
39+
head(next.size()?(&next[_length]):NULL),
3940
id_to_value(_length),
4041
bucket(_bucket), id(_id) { }
4142

@@ -47,11 +48,8 @@ namespace boost {
4748
//check if i is the end of the bucket list
4849
if ( next_node != invalid_value() )
4950
prev[next_node] = prev_node;
50-
//check if i is the begin of the bucket list
51-
if ( prev_node != invalid_value() )
52-
next[prev_node] = next_node;
53-
else //need update head of current bucket list
54-
head[ bucket[x] ] = next_node;
51+
//update predecessor
52+
next[prev_node] = next_node;
5553
}
5654

5755
void push(const value_type& x) {
@@ -78,21 +76,22 @@ namespace boost {
7876

7977
class stack {
8078
public:
81-
stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v,
82-
const ValueIndexMap& _id)
79+
stack(bucket_type _bucket_id, typename Iter::value_type* h,
80+
Iter n, Iter p, IndexValueMap v, const ValueIndexMap& _id)
8381
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v), id(_id) {}
8482

8583
// Avoid using default arg for ValueIndexMap so that the default
8684
// constructor of the ValueIndexMap is not required if not used.
87-
stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v)
85+
stack(bucket_type _bucket_id, typename Iter::value_type* h,
86+
Iter n, Iter p, IndexValueMap v)
8887
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v) {}
8988

9089
void push(const value_type& x) {
9190
const size_type new_head = get(id, x);
9291
const size_type current = head[bucket_id];
9392
if ( current != invalid_value() )
9493
prev[current] = new_head;
95-
prev[new_head] = invalid_value();
94+
prev[new_head] = bucket_id + (head - next);
9695
next[new_head] = current;
9796
head[bucket_id] = new_head;
9897
}
@@ -101,7 +100,7 @@ namespace boost {
101100
size_type next_node = next[current];
102101
head[bucket_id] = next_node;
103102
if ( next_node != invalid_value() )
104-
prev[next_node] = invalid_value();
103+
prev[next_node] = bucket_id + (head - next);
105104
}
106105
value_type& top() { return value[ head[bucket_id] ]; }
107106
const value_type& top() const { return value[ head[bucket_id] ]; }
@@ -116,14 +115,14 @@ namespace boost {
116115
};
117116

118117
stack operator[](const bucket_type& i) {
119-
assert(i < head.size());
120-
return stack(i, head.begin(), next.begin(), prev.begin(),
118+
assert(i < next.size());
119+
return stack(i, head, next.begin(), prev.begin(),
121120
id_to_value.begin(), id);
122121
}
123122
protected:
124-
std::vector<size_type> head;
125123
std::vector<size_type> next;
126124
std::vector<size_type> prev;
125+
typename std::vector<size_type>::value_type* head;
127126
std::vector<value_type> id_to_value;
128127
Bucket bucket;
129128
ValueIndexMap id;

0 commit comments

Comments
 (0)