clang-tidy points out that `UserMessageIterator` has a user-defined
destructor, but default copy constructors and assignment operators.
This is indeed dangerous: if we have an error in `_mSavedLibError` and
the `UserMessageIterator` object gets copied, the error will be released
multiple times, causing a use-after-free.
The most obvious fix would be to explicitly delete the default
definitions of the copy constructors and assignment operators. But
I think it's a bit more idiomatic C++ to use some RAII type to manage
the error. In this case `std::unique_ptr` is a good fit. This lets us
delete the user-defined destructor. The copy constructors and
assignment operators are implicitly deleted, since `unique_ptr` doesn't
have them.
Change-Id: Ia78bbda6a342192cb35c1b6963d3ad7fccaa7c3f
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/11844
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
- ~UserMessageIterator()
- {
- this->_resetError();
- }
-
void next(bt2::ConstMessageArray& messages)
{
/* Any saved error? Now is the time to throw */
if (G_UNLIKELY(_mExcToThrowType != _ExcToThrowType::NONE)) {
/* Move `_mSavedLibError`, if any, as current thread error */
if (_mSavedLibError) {
void next(bt2::ConstMessageArray& messages)
{
/* Any saved error? Now is the time to throw */
if (G_UNLIKELY(_mExcToThrowType != _ExcToThrowType::NONE)) {
/* Move `_mSavedLibError`, if any, as current thread error */
if (_mSavedLibError) {
- BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(_mSavedLibError);
+ bt_current_thread_move_error(_mSavedLibError.release());
}
/* Throw the corresponding exception */
}
/* Throw the corresponding exception */
"An error occurred, but there are {} messages to return: delaying the error reporting.",
messages.length());
BT_ASSERT(!_mSavedLibError);
"An error occurred, but there are {} messages to return: delaying the error reporting.",
messages.length());
BT_ASSERT(!_mSavedLibError);
- _mSavedLibError = bt_current_thread_take_error();
+ _mSavedLibError.reset(bt_current_thread_take_error());
void _resetError() noexcept
{
_mExcToThrowType = _ExcToThrowType::NONE;
void _resetError() noexcept
{
_mExcToThrowType = _ExcToThrowType::NONE;
-
- if (_mSavedLibError) {
- bt_error_release(_mSavedLibError);
- }
+ _mSavedLibError.reset();
}
SelfMessageIterator _mSelfMsgIter;
}
SelfMessageIterator _mSelfMsgIter;
* It also saves the type of the exception to throw the next time.
*/
_ExcToThrowType _mExcToThrowType = _ExcToThrowType::NONE;
* It also saves the type of the exception to throw the next time.
*/
_ExcToThrowType _mExcToThrowType = _ExcToThrowType::NONE;
- const bt_error *_mSavedLibError = nullptr;
+
+ struct LibErrorDeleter final
+ {
+ void operator()(const bt_error * const error) const noexcept
+ {
+ bt_error_release(error);
+ }
+ };
+
+ std::unique_ptr<const bt_error, LibErrorDeleter> _mSavedLibError;
protected:
bt2c::Logger _mLogger;
protected:
bt2c::Logger _mLogger;