consensus: fix maybe uninitialized CTxMemPool::GetIter() #20797
Conversation
| @@ -880,7 +880,7 @@ Optional<CTxMemPool::txiter> CTxMemPool::GetIter(const uint256& txid) const | |||
| { | |||
| auto it = mapTx.find(txid); | |||
| if (it != mapTx.end()) return it; | |||
| return Optional<txiter>{}; | |||
| return Optional<txiter>{nullptr}; | |||
There was a problem hiding this comment.
Mind to explain how this fix works and why it is correct. I might be missing something, but https://en.cppreference.com/w/cpp/utility/optional/optional doesn't list a constructor that accepts a nullptr.
Unrelated, this method was added in faa1a74 (by me) to not expose mapTx, which is an implementation detail. Though, I realize that GetIter doesn't follow the iterator-scheme (find(), end(), begin(), ...). It may be better to hide the mapTx from outside by exposing Find(), End(), Begin(), which are internally forwarded to the corresponding mapTx methods.
There was a problem hiding this comment.
This should be return nullopt or return Optional<txiter>{nullopt} I think. I'd assume nullptr gets coerced to a txiter?
There was a problem hiding this comment.
I don't get it why there is a warning in the first place. std::optional<txiter>{} could call the optional's constructor that "Constructs an object that does not contain a value", same as std::optional<txiter>{std::nullopt}. Would std::optional<txiter>() fix it?
Clang has no -Wmaybe-uninitialized option, it must be gcc that prints this (OP mentions clang)?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 may be related.
There was a problem hiding this comment.
I don't get it why there is a warning in the first place.
GCC's warnings are very brittle, which can be seen from the open issues in their bug tracker. Personally I've seen many gcc warnings appear/disappear depending on optimization level or trivial code restructuring. It would be a full time job to report all these inconsistencies on the bugtracker.
|
Thanks for the feedback. I had to be away from the computer for a couple hours but will look carefully at a better solution this afternoon. |
|
I saw this warning three times during the morning. On returning this afternoon, I could no longer reproduce, idem after rebooting, and after apt updating. Closing with apologies for the noise. |
Seen while compiling with clang: