From: Simon Marchi Date: Mon, 9 Sep 2024 19:44:08 +0000 (-0400) Subject: ccv: output reference clock class on error X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=0c9a26f3293744b99b6fe06d3847559c6c176a31;p=babeltrace.git ccv: output reference clock class on error On validation error, always output the clock class used as a reference. In combination with the error type, the callers will be able to inspect the reference clock class to obtain the details needed to generate the appropriate error message. The goal of this change is to avoid having to add many output parameters when adding support for MIP 1, keeping things simpler. Change-Id: I032cc706d586134a35aabd14d9fb00611b430ad0 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/12782 Reviewed-by: Philippe Proulx Tested-by: jenkins --- diff --git a/src/clock-correlation-validator/clock-correlation-validator.cpp b/src/clock-correlation-validator/clock-correlation-validator.cpp index b5c9c21b..a0f015a5 100644 --- a/src/clock-correlation-validator/clock-correlation-validator.cpp +++ b/src/clock-correlation-validator/clock-correlation-validator.cpp @@ -36,27 +36,27 @@ void ClockCorrelationValidator::_validate(const bt2::ConstMessage msg) case PropsExpectation::Unset: /* * This is the first analysis of a message with a clock - * snapshot: record the properties of that clock, against which - * we'll compare the clock properties of the following messages. + * snapshot: record the clock class against which we'll compare + * the clock class properties of the following messages. */ - if (!clockCls) { - _mExpectation = PropsExpectation::None; - } else if (clockCls->origin().isUnixEpoch()) { - _mExpectation = PropsExpectation::OriginUnix; - } else if (const auto uuid = clockCls->uuid()) { - _mExpectation = PropsExpectation::OriginOtherUuid; - _mUuid = *uuid; + if (clockCls) { + _mRefClockClass = clockCls->shared(); + + if (clockCls->origin().isUnixEpoch()) { + _mExpectation = PropsExpectation::OriginUnix; + } else if (const auto uuid = clockCls->uuid()) { + _mExpectation = PropsExpectation::OriginOtherUuid; + } else { + _mExpectation = PropsExpectation::OriginOtherNoUuid; + } } else { - _mExpectation = PropsExpectation::OriginOtherNoUuid; - _mClockClass = clockCls->shared(); + _mExpectation = PropsExpectation::None; } - break; case PropsExpectation::None: if (clockCls) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingNoClockClassGotOne, - bt2s::nullopt, *clockCls, {}, streamCls}; @@ -67,18 +67,14 @@ void ClockCorrelationValidator::_validate(const bt2::ConstMessage msg) case PropsExpectation::OriginUnix: if (!clockCls) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginUnixGotNone, - bt2s::nullopt, - {}, {}, + *_mRefClockClass, streamCls}; } if (!clockCls->origin().isUnixEpoch()) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginUnixGotOther, - bt2s::nullopt, - *clockCls, - {}, - streamCls}; + *clockCls, *_mRefClockClass, streamCls}; } break; @@ -87,37 +83,27 @@ void ClockCorrelationValidator::_validate(const bt2::ConstMessage msg) { if (!clockCls) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginUuidGotNone, - bt2s::nullopt, - {}, {}, + *_mRefClockClass, streamCls}; } if (clockCls->origin().isUnixEpoch()) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginUuidGotUnix, - bt2s::nullopt, - *clockCls, - {}, - streamCls}; + *clockCls, *_mRefClockClass, streamCls}; } const auto uuid = clockCls->uuid(); if (!uuid) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginUuidGotNoUuid, - bt2s::nullopt, - *clockCls, - {}, - streamCls}; + *clockCls, *_mRefClockClass, streamCls}; } - if (*uuid != _mUuid) { + if (*uuid != *_mRefClockClass->uuid()) { throw ClockCorrelationError { - ClockCorrelationError::Type::ExpectingOriginUuidGotOtherUuid, - _mUuid, - *clockCls, - {}, - streamCls}; + ClockCorrelationError::Type::ExpectingOriginUuidGotOtherUuid, *clockCls, + *_mRefClockClass, streamCls}; } break; @@ -126,15 +112,14 @@ void ClockCorrelationValidator::_validate(const bt2::ConstMessage msg) case PropsExpectation::OriginOtherNoUuid: if (!clockCls) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginNoUuidGotNone, - bt2s::nullopt, - {}, {}, + *_mRefClockClass, streamCls}; } - if (clockCls->libObjPtr() != _mClockClass->libObjPtr()) { + if (clockCls->libObjPtr() != _mRefClockClass->libObjPtr()) { throw ClockCorrelationError {ClockCorrelationError::Type::ExpectingOriginNoUuidGotOther, - bt2s::nullopt, *clockCls, *_mClockClass, streamCls}; + *clockCls, *_mRefClockClass, streamCls}; } break; @@ -158,9 +143,9 @@ bt_clock_correlation_validator *bt_clock_correlation_validator_create() noexcept bool bt_clock_correlation_validator_validate_message( bt_clock_correlation_validator * const validator, const bt_message * const msg, - bt_clock_correlation_validator_error_type * const type, bt_uuid * const expectedUuidOut, + bt_clock_correlation_validator_error_type * const type, const bt_clock_class ** const actualClockClsOut, - const bt_clock_class ** const expectedClockClsOut) noexcept + const bt_clock_class ** const refClockClsOut) noexcept { try { reinterpret_cast(validator)->validate(bt2::wrap(msg)); @@ -168,22 +153,16 @@ bool bt_clock_correlation_validator_validate_message( } catch (const bt2ccv::ClockCorrelationError& error) { *type = static_cast(error.type()); - if (error.expectedUuid()) { - *expectedUuidOut = error.expectedUuid()->data(); - } else { - *expectedUuidOut = nullptr; - } - if (error.actualClockCls()) { *actualClockClsOut = error.actualClockCls()->libObjPtr(); } else { *actualClockClsOut = nullptr; } - if (error.expectedClockCls()) { - *expectedClockClsOut = error.expectedClockCls()->libObjPtr(); + if (error.refClockCls()) { + *refClockClsOut = error.refClockCls()->libObjPtr(); } else { - *expectedClockClsOut = nullptr; + *refClockClsOut = nullptr; } return false; diff --git a/src/clock-correlation-validator/clock-correlation-validator.h b/src/clock-correlation-validator/clock-correlation-validator.h index 20c1509d..bed3dbad 100644 --- a/src/clock-correlation-validator/clock-correlation-validator.h +++ b/src/clock-correlation-validator/clock-correlation-validator.h @@ -40,9 +40,8 @@ bool bt_clock_correlation_validator_validate_message( struct bt_clock_correlation_validator *validator, const struct bt_message *msg, enum bt_clock_correlation_validator_error_type *type, - bt_uuid *expected_uuid, const struct bt_clock_class ** const actual_clock_cls, - const struct bt_clock_class ** const expected_clock_cls) BT_NOEXCEPT; + const struct bt_clock_class ** const ref_clock_cls) BT_NOEXCEPT; void bt_clock_correlation_validator_destroy( struct bt_clock_correlation_validator *validator) BT_NOEXCEPT; diff --git a/src/clock-correlation-validator/clock-correlation-validator.hpp b/src/clock-correlation-validator/clock-correlation-validator.hpp index 94ca4e00..0e066428 100644 --- a/src/clock-correlation-validator/clock-correlation-validator.hpp +++ b/src/clock-correlation-validator/clock-correlation-validator.hpp @@ -39,13 +39,12 @@ public: }; explicit ClockCorrelationError( - Type type, const bt2s::optional expectedUuid, - const bt2::OptionalBorrowedObject actualClockCls, - const bt2::OptionalBorrowedObject expectedClockCls, + Type type, const bt2::OptionalBorrowedObject actualClockCls, + const bt2::OptionalBorrowedObject refClockCls, const bt2::OptionalBorrowedObject streamCls) noexcept : std::runtime_error {"Clock classes are not correlatable"}, - _mType {type}, _mExpectedUuid {expectedUuid}, _mActualClockCls {actualClockCls}, - _mExpectedClockCls {expectedClockCls}, _mStreamCls {streamCls} + _mType {type}, _mActualClockCls {actualClockCls}, _mRefClockCls {refClockCls}, + _mStreamCls {streamCls} { } @@ -55,19 +54,14 @@ public: return _mType; } - bt2s::optional expectedUuid() const noexcept - { - return _mExpectedUuid; - } - bt2::OptionalBorrowedObject actualClockCls() const noexcept { return _mActualClockCls; } - bt2::OptionalBorrowedObject expectedClockCls() const noexcept + bt2::OptionalBorrowedObject refClockCls() const noexcept { - return _mExpectedClockCls; + return _mRefClockCls; } bt2::OptionalBorrowedObject streamCls() const noexcept @@ -77,9 +71,8 @@ public: private: Type _mType; - bt2s::optional _mExpectedUuid; bt2::OptionalBorrowedObject _mActualClockCls; - bt2::OptionalBorrowedObject _mExpectedClockCls; + bt2::OptionalBorrowedObject _mRefClockCls; bt2::OptionalBorrowedObject _mStreamCls; }; @@ -120,30 +113,14 @@ private: PropsExpectation _mExpectation = PropsExpectation::Unset; /* - * Expected UUID of the clock, if `_mExpectation` is - * `PropsExpectation::ORIGIN_OTHER_UUID`. - * - * If the origin of the clock is the Unix epoch, then the UUID is - * irrelevant because the clock will have a correlation with other - * clocks having the same origin. - */ - bt2c::Uuid _mUuid; - - /* - * Expected clock class, if `_mExpectation` is - * `ClockExpectation::ORIGIN_OTHER_NO_UUID`. - * - * If the first analyzed clock class has an unknown origin and no - * UUID, then all subsequent analyzed clock classes must be the same - * instance. + * Reference clock class: the clock class used to set expectations. * * To make sure that the clock class pointed to by this member * doesn't get freed and another one reallocated at the same - * address, which could potentially bypass the clock expectation - * check, we keep a strong reference, ensuring that the clock class - * lives at least as long as the owner of this validator. + * address, keep a strong reference, ensuring that it lives at least + * as long as the owner of this validator. */ - bt2::ConstClockClass::Shared _mClockClass; + bt2::ConstClockClass::Shared _mRefClockClass; }; } /* namespace bt2ccv */ diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c index f7ecc47c..d27f3284 100644 --- a/src/lib/graph/iterator.c +++ b/src/lib/graph/iterator.c @@ -686,14 +686,12 @@ void assert_post_dev_clock_classes_are_compatible_one( const struct bt_message *msg) { enum bt_clock_correlation_validator_error_type type; - bt_uuid expected_uuid; const bt_clock_class *actual_clock_cls; - const bt_clock_class *expected_clock_cls; + const bt_clock_class *ref_clock_cls; if (!bt_clock_correlation_validator_validate_message( iterator->correlation_validator, msg, &type, - &expected_uuid, &actual_clock_cls, - &expected_clock_cls)) { + &actual_clock_cls, &ref_clock_cls)) { switch (type) { case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_NO_CLOCK_CLASS_GOT_ONE: BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, @@ -725,8 +723,9 @@ void assert_post_dev_clock_classes_are_compatible_one( case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UUID_GOT_OTHER_UUID: BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, "clock-class-has-expected-uuid", false, - "Expecting a clock class with UUID, got one with a different UUID: %![cc-]+K, expected-uuid=%!u", - actual_clock_cls, expected_uuid); + "Expecting a clock class with UUID, got one with a different UUID: " + "%![cc-]+K, expected-uuid=%!u", + actual_clock_cls, bt_clock_class_get_uuid(ref_clock_cls)); case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_NO_UUID_GOT_NONE: BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, @@ -736,7 +735,7 @@ void assert_post_dev_clock_classes_are_compatible_one( BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, "clock-class-is-expected", false, "Unexpected clock class: %![expected-cc-]+K, %![actual-cc-]+K", - expected_clock_cls, actual_clock_cls); + ref_clock_cls, actual_clock_cls); } bt_common_abort(); diff --git a/src/plugins/utils/muxer/msg-iter.cpp b/src/plugins/utils/muxer/msg-iter.cpp index 0fc9757e..2da2b8d0 100644 --- a/src/plugins/utils/muxer/msg-iter.cpp +++ b/src/plugins/utils/muxer/msg-iter.cpp @@ -251,6 +251,7 @@ void MsgIter::_validateMsgClkCls(const bt2::ConstMessage msg) using Type = bt2ccv::ClockCorrelationError::Type; const auto actualClkCls = error.actualClockCls(); + const auto refClkCls = error.refClockCls(); switch (error.type()) { case Type::ExpectingNoClockClassGotOne: @@ -296,7 +297,7 @@ void MsgIter::_validateMsgClkCls(const bt2::ConstMessage msg) bt2::Error, "Expecting a clock class with a specific UUID, but got one with a different UUID: " "clock-class-addr={}, clock-class-name={}, expected-uuid=\"{}\", uuid=\"{}\"", - fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name(), *error.expectedUuid(), + fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name(), *refClkCls->uuid(), *actualClkCls->uuid()); case Type::ExpectingOriginNoUuidGotOther: @@ -305,7 +306,7 @@ void MsgIter::_validateMsgClkCls(const bt2::ConstMessage msg) "Unexpected clock class: " "expected-clock-class-addr={}, expected-clock-class-name={}, " "actual-clock-class-addr={}, actual-clock-class-name={}", - fmt::ptr(error.expectedClockCls()->libObjPtr()), error.expectedClockCls()->name(), + fmt::ptr(refClkCls->libObjPtr()), refClkCls->name(), fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name()); } }