Skip to content

Commit 9e859b7

Browse files
A few improvements to DPL InputRecord (#3061)
- fix the end iterator instance of part iterator - making const what should be const - bounds check before getNofParts callback to check against span size - init InputRecord from InputSpan by move (which was intended but there was an unwanted copy) - making InputRecord::get<T>(DataRef) a static method This method actually only requires the pointers from the DataRef argument, no concrete object or state is needed. At some point this can be also seperated from InputRecord, thus separating data and serialization method.
1 parent cbfedce commit 9e859b7

4 files changed

Lines changed: 29 additions & 8 deletions

File tree

Framework/Core/include/Framework/InputRecord.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class InputRecord
183183

184184
size_t getNofParts(int pos) const
185185
{
186-
if (pos < 0) {
186+
if (pos < 0 || pos >= mSpan.size()) {
187187
return 0;
188188
}
189189
return mSpan.getNofParts(pos);
@@ -226,7 +226,7 @@ class InputRecord
226226
/// incoming data. See @ref Inputrecord class description for supported types.
227227
/// @param ref DataRef with pointers to input spec, header, and payload
228228
template <typename T>
229-
decltype(auto) get(DataRef ref) const
229+
static decltype(auto) get(DataRef ref)
230230
{
231231
if constexpr (std::is_same<T, std::string>::value) {
232232
// substitution for std::string
@@ -468,7 +468,7 @@ class InputRecord
468468
if (mParent->isValid(mPosition)) {
469469
mElement = mParent->getByPos(mPosition);
470470
} else {
471-
(*this)++;
471+
++(*this);
472472
}
473473
}
474474
}
@@ -499,17 +499,17 @@ class InputRecord
499499
return copy;
500500
}
501501
// return reference
502-
reference operator*()
502+
reference operator*() const
503503
{
504504
return mElement;
505505
}
506506
// comparison
507-
bool operator==(const SelfType& rh)
507+
bool operator==(const SelfType& rh) const
508508
{
509509
return mPosition == rh.mPosition;
510510
}
511511
// comparison
512-
bool operator!=(const SelfType& rh)
512+
bool operator!=(const SelfType& rh) const
513513
{
514514
return mPosition != rh.mPosition;
515515
}
@@ -588,7 +588,10 @@ class InputRecord
588588
/// Check if slot is valid, index of part is not used
589589
bool isValid(size_t = 0) const
590590
{
591-
return this->parent()->isValid(this->position());
591+
if (this->position() < this->parent()->size()) {
592+
return this->parent()->isValid(this->position());
593+
}
594+
return false;
592595
}
593596

594597
/// Get number of parts in input slot

Framework/Core/include/Framework/InputSpan.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ namespace framework
2323
class InputSpan
2424
{
2525
public:
26+
InputSpan() = delete;
27+
InputSpan(InputSpan const&) = delete;
28+
InputSpan(InputSpan&&) = default;
29+
2630
/// @a getter is the mapping between an element of the span referred by
2731
/// index and the buffer associated.
2832
/// @a size is the number of elements in the span.
@@ -60,6 +64,9 @@ class InputSpan
6064
/// @a number of parts in the i-th element of the InputSpan
6165
size_t getNofParts(size_t i) const
6266
{
67+
if (i >= mSize) {
68+
return 0;
69+
}
6370
if (!mNofPartsGetter) {
6471
return 1;
6572
}

Framework/Core/src/InputRecord.cxx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace framework
3434
InputRecord::InputRecord(std::vector<InputRoute> const& inputsSchema,
3535
InputSpan&& span)
3636
: mInputsSchema{inputsSchema},
37-
mSpan{span}
37+
mSpan{std::move(span)}
3838
{
3939
}
4040

@@ -71,6 +71,9 @@ bool InputRecord::isValid(char const* s) const
7171

7272
bool InputRecord::isValid(int s) const
7373
{
74+
if (s >= size()) {
75+
return false;
76+
}
7477
DataRef ref = getByPos(s);
7578
if (ref.header == nullptr || ref.payload == nullptr) {
7679
return false;

Framework/Core/test/test_InputRecord.cxx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ BOOST_AUTO_TEST_CASE(TestInputRecord)
144144
// check if invalid slots are filtered out by the iterator
145145
BOOST_CHECK(position != 2);
146146
}
147+
148+
// the 2-level iterator to access inputs and their parts
149+
// all inputs have 1 part, we check the first input
150+
BOOST_CHECK(record.begin().size() == 1);
151+
// the end-instance of the inputs has no parts
152+
BOOST_CHECK(record.end().size() == 0);
153+
// thus there is no element and begin == end
154+
BOOST_CHECK(record.end().begin() == record.end().end());
147155
}
148156

149157
// TODO:

0 commit comments

Comments
 (0)