This one is serious because it impacts actual runtime, not just tests.
- R165:548c54a1b25c: Fix leaking of requests in ItemRetriever::exec()
ctest -R itemretrievertest in an ASAN build
I don't really like shared_ptr for things like this. To me shared_ptr means "ownership is unclear" (when misused) or "ownership is shared between multiple objects, we need refcounting so that the last user deletes it".
This isn't the case here. ItemRetriever creates the requests, ItemRetrievalManager processes them (but never owns them), and then we need to delete them after use.
A refcount is unnecessary (i.e. overkill) and makes it harder to find out the ownership situation by reading the code.
The owner is clear and unique: it's the requests list. Hence the delete after the removeOne.
Good suggestion, thanks.
A for loop better than find_if? What makes you say that?
It's general consensus that using an algorithm is always better than writing a for loop.