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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/12782
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/clock-correlation-validator/clock-correlation-validator.cpp b/src/clock-correlation-validator/clock-correlation-validator.cpp
index b5c9c21b74431eb3098ac52c4f244b064c36385e..a0f015a5d51d21f58fe08c6c61709778f1a6d696 100644 (file)
--- 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<bt2ccv::ClockCorrelationValidator *>(validator)->validate(bt2::wrap(msg));
@@ -168,22 +153,16 @@ bool bt_clock_correlation_validator_validate_message(
     } catch (const bt2ccv::ClockCorrelationError& error) {
         *type = static_cast<bt_clock_correlation_validator_error_type>(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 20c1509ddb1feca9b891d08b5114bad3ed2b387a..bed3dbad7fdbe94205b8fee62ecf45ee9aff356a 100644 (file)
--- 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 94ca4e00ebf9ae7c41ddb51601b5c794b4014191..0e0664289a3f3ebc8c344ab71acfc0fcaa5535cf 100644 (file)
--- 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<bt2c::UuidView> expectedUuid,
-        const bt2::OptionalBorrowedObject<bt2::ConstClockClass> actualClockCls,
-        const bt2::OptionalBorrowedObject<bt2::ConstClockClass> expectedClockCls,
+        Type type, const bt2::OptionalBorrowedObject<bt2::ConstClockClass> actualClockCls,
+        const bt2::OptionalBorrowedObject<bt2::ConstClockClass> refClockCls,
         const bt2::OptionalBorrowedObject<bt2::ConstStreamClass> 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<bt2c::UuidView> expectedUuid() const noexcept
-    {
-        return _mExpectedUuid;
-    }
-
     bt2::OptionalBorrowedObject<bt2::ConstClockClass> actualClockCls() const noexcept
     {
         return _mActualClockCls;
     }
 
-    bt2::OptionalBorrowedObject<bt2::ConstClockClass> expectedClockCls() const noexcept
+    bt2::OptionalBorrowedObject<bt2::ConstClockClass> refClockCls() const noexcept
     {
-        return _mExpectedClockCls;
+        return _mRefClockCls;
     }
 
     bt2::OptionalBorrowedObject<bt2::ConstStreamClass> streamCls() const noexcept
@@ -77,9 +71,8 @@ public:
 
 private:
     Type _mType;
-    bt2s::optional<bt2c::UuidView> _mExpectedUuid;
     bt2::OptionalBorrowedObject<bt2::ConstClockClass> _mActualClockCls;
-    bt2::OptionalBorrowedObject<bt2::ConstClockClass> _mExpectedClockCls;
+    bt2::OptionalBorrowedObject<bt2::ConstClockClass> _mRefClockCls;
     bt2::OptionalBorrowedObject<bt2::ConstStreamClass> _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 f7ecc47c3c1636e98ab08cede13111977e94ef77..d27f328423d90c03551e62d9eac0ee0401c88b01 100644 (file)
--- 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 0fc9757e748dcaacee3b4ed22f0b7a71dd317418..2da2b8d0369dcf4c7fb79f5d133d8c4feb8ebfde 100644 (file)
--- 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());
         }
     }