sink.ctf.fs: add more comments in code where it's not straightforward
authorPhilippe Proulx <eeppeliteloop@gmail.com>
Tue, 4 Jun 2019 20:58:26 +0000 (16:58 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Thu, 6 Jun 2019 21:19:12 +0000 (17:19 -0400)
This patch adds a few comments in the code of `sink.ctf.fs` to help
future contributors without deep knowledge of CTF.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ibaa9ae27adb42f76329d0eab5a321368f452fc56
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1376
Tested-by: jenkins
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
plugins/ctf/fs-sink/fs-sink-stream.h
plugins/ctf/fs-sink/fs-sink.c
plugins/ctf/fs-sink/fs-sink.h

index 9f48ce85d3669e37db5e75bbcb54159c51091b77..ad6fdcfd014283b7ab90d8bec1af451781fc3cbe 100644 (file)
@@ -46,39 +46,126 @@ struct fs_sink_stream {
 
        struct fs_sink_ctf_stream_class *sc;
 
+       /* Current packet's state */
        struct {
+               /*
+                * True if we're, for this stream, within an opened
+                * packet (got a packet beginning message, but no
+                * packet end message yet).
+                */
                bool is_open;
+
+               /*
+                * Current beginning default clock snapshot for the
+                * current packet (`UINT64_C(-1)` if not set).
+                */
                uint64_t beginning_cs;
+
+               /*
+                * Current end default clock snapshot for the current
+                * packet (`UINT64_C(-1)` if not set).
+                */
                uint64_t end_cs;
+
+               /*
+                * Current packet's content size (bits) for the current
+                * packet.
+                */
                uint64_t content_size;
+
+               /*
+                * Current packet's total size (bits) for the current
+                * packet.
+                */
                uint64_t total_size;
+
+               /*
+                * Discarded events (free running) counter for the
+                * current packet.
+                */
                uint64_t discarded_events_counter;
+
+               /* Sequence number (free running) of the current packet */
                uint64_t seq_num;
+
+               /*
+                * Offset of the packet context structure within the
+                * current packet (bits).
+                */
                uint64_t context_offset_bits;
 
                /* Owned by this */
                const bt_packet *packet;
        } packet_state;
 
+       /* Previous packet's state */
        struct {
+               /* End default clock snapshot (`UINT64_C(-1)` if not set) */
                uint64_t end_cs;
+
+               /* Discarded events (free running) counter */
                uint64_t discarded_events_counter;
+
+               /* Sequence number (free running) */
                uint64_t seq_num;
        } prev_packet_state;
 
+       /* State to handle discarded events */
        struct {
+               /*
+                * True if we're in the time range given by a previously
+                * received discarded events message. In this case,
+                * `beginning_cs` and `end_cs` below contain the
+                * beginning and end clock snapshots for this range.
+                *
+                * This is used to validate that, when receiving a
+                * packet end message, the current discarded events time
+                * range matches what's expected for CTF 1.8, that is:
+                *
+                * * Its beginning time is the previous packet's end
+                *   time (or the current packet's beginning time if
+                *   this is the first packet).
+                *
+                * * Its end time is the current packet's end time.
+                */
                bool in_range;
+
+               /*
+                * Beginning and end times of the time range given by a
+                * previously received discarded events message.
+                */
                uint64_t beginning_cs;
                uint64_t end_cs;
        } discarded_events_state;
 
+       /* State to handle discarded packets */
        struct {
+               /*
+                * True if we're in the time range given by a previously
+                * received discarded packets message. In this case,
+                * `beginning_cs` and `end_cs` below contain the
+                * beginning and end clock snapshots for this range.
+                *
+                * This is used to validate that, when receiving a
+                * packet beginning message, the current discarded
+                * packets time range matches what's expected for CTF
+                * 1.8, that is:
+                *
+                * * Its beginning time is the previous packet's end
+                *   time.
+                *
+                * * Its end time is the current packet's beginning
+                *   time.
+                */
                bool in_range;
+
+               /*
+                * Beginning and end times of the time range given by a
+                * previously received discarded packets message.
+                */
                uint64_t beginning_cs;
                uint64_t end_cs;
        } discarded_packets_state;
-
-       bool in_discarded_events_range;
 };
 
 BT_HIDDEN
index 5f04496f0d523db891c8dacf11931daf7bed91fa..9cc438830b1ddf434f228c92eb36b8782ca0500e 100644 (file)
@@ -326,17 +326,38 @@ bt_self_component_status handle_packet_beginning_msg(
                BT_ASSERT(cs);
        }
 
+       /*
+        * If we previously received a discarded events message with
+        * a time range, make sure that its beginning time matches what's
+        * expected for CTF 1.8, that is:
+        *
+        * * Its beginning time is the previous packet's end
+        *   time (or the current packet's beginning time if
+        *   this is the first packet).
+        *
+        * We check this here instead of in handle_packet_end_msg()
+        * because we want to catch any incompatible message as early as
+        * possible to report the error.
+        *
+        * Validation of the discarded events message's end time is
+        * performed in handle_packet_end_msg().
+        */
        if (stream->discarded_events_state.in_range) {
                uint64_t expected_cs;
 
+               /*
+                * `stream->discarded_events_state.in_range` is only set
+                * when the stream class's discarded events have a time
+                * range.
+                *
+                * It is required that the packet beginning and end
+                * messages for this stream class have times when
+                * discarded events have a time range.
+                */
                BT_ASSERT(stream->sc->discarded_events_has_ts);
                BT_ASSERT(stream->sc->packets_have_ts_begin);
                BT_ASSERT(stream->sc->packets_have_ts_end);
 
-               /*
-                * Make sure that the current discarded events range's
-                * beginning time matches what's expected for CTF 1.8.
-                */
                if (stream->prev_packet_state.end_cs == UINT64_C(-1)) {
                        /* We're opening the first packet */
                        expected_cs = bt_clock_snapshot_get_value(cs);
@@ -364,17 +385,36 @@ bt_self_component_status handle_packet_beginning_msg(
                }
        }
 
+       /*
+        * If we previously received a discarded packets message with a
+        * time range, make sure that its beginning and end times match
+        * what's expected for CTF 1.8, that is:
+        *
+        * * Its beginning time is the previous packet's end time.
+        *
+        * * Its end time is the current packet's beginning time.
+        */
        if (stream->discarded_packets_state.in_range) {
                uint64_t expected_end_cs;
 
+               /*
+                * `stream->discarded_packets_state.in_range` is only
+                * set when the stream class's discarded packets have a
+                * time range.
+                *
+                * It is required that the packet beginning and end
+                * messages for this stream class have times when
+                * discarded packets have a time range.
+                */
                BT_ASSERT(stream->sc->discarded_packets_has_ts);
                BT_ASSERT(stream->sc->packets_have_ts_begin);
                BT_ASSERT(stream->sc->packets_have_ts_end);
 
                /*
-                * Make sure that the current discarded packets range's
-                * beginning and end times match what's expected for CTF
-                * 1.8.
+                * It is not supported to have a discarded packets
+                * message _before_ the first packet: we cannot validate
+                * that its beginning time is compatible with CTF 1.8 in
+                * this case.
                 */
                if (stream->prev_packet_state.end_cs == UINT64_C(-1)) {
                        BT_LOGE("Incompatible discarded packets message "
@@ -431,9 +471,13 @@ bt_self_component_status handle_packet_beginning_msg(
                }
        }
 
-       if (stream->sc->discarded_packets_has_ts) {
-               stream->discarded_packets_state.in_range = false;
-       }
+       /*
+        * We're not in a discarded packets time range anymore since we
+        * require that the discarded packets time ranges go from one
+        * packet's end time to the next packet's beginning time, and
+        * we're handling a packet beginning message here.
+        */
+       stream->discarded_packets_state.in_range = false;
 
        ret = fs_sink_stream_open_packet(stream, cs, ir_packet);
        if (ret) {
@@ -469,17 +513,32 @@ bt_self_component_status handle_packet_end_msg(
                BT_ASSERT(cs);
        }
 
+       /*
+        * If we previously received a discarded events message with
+        * a time range, make sure that its end time matches what's
+        * expected for CTF 1.8, that is:
+        *
+        * * Its end time is the current packet's end time.
+        *
+        * Validation of the discarded events message's beginning time
+        * is performed in handle_packet_beginning_msg().
+        */
        if (stream->discarded_events_state.in_range) {
                uint64_t expected_cs;
 
+               /*
+                * `stream->discarded_events_state.in_range` is only set
+                * when the stream class's discarded events have a time
+                * range.
+                *
+                * It is required that the packet beginning and end
+                * messages for this stream class have times when
+                * discarded events have a time range.
+                */
                BT_ASSERT(stream->sc->discarded_events_has_ts);
                BT_ASSERT(stream->sc->packets_have_ts_begin);
                BT_ASSERT(stream->sc->packets_have_ts_end);
 
-               /*
-                * Make sure that the current discarded events range's
-                * end time matches what's expected for CTF 1.8.
-                */
                expected_cs = bt_clock_snapshot_get_value(cs);
 
                if (stream->discarded_events_state.end_cs != expected_cs) {
@@ -507,9 +566,13 @@ bt_self_component_status handle_packet_end_msg(
                goto end;
        }
 
-       if (stream->sc->discarded_events_has_ts) {
-               stream->discarded_events_state.in_range = false;
-       }
+       /*
+        * We're not in a discarded events time range anymore since we
+        * require that the discarded events time ranges go from one
+        * packet's end time to the next packet's end time, and we're
+        * handling a packet end message here.
+        */
+       stream->discarded_events_state.in_range = false;
 
 end:
        return status;
@@ -660,6 +723,14 @@ bt_self_component_status handle_discarded_events_msg(
                goto end;
        }
 
+       /*
+        * If we're currently in an opened packet (got a packet
+        * beginning message, but no packet end message yet), we do not
+        * support having a discarded events message with a time range
+        * because we require that the discarded events message's time
+        * range go from a packet's end time to the next packet's end
+        * time.
+        */
        if (stream->packet_state.is_open &&
                        stream->sc->discarded_events_has_ts) {
                BT_LOGE("Unsupported discarded events message with "
@@ -676,11 +747,18 @@ bt_self_component_status handle_discarded_events_msg(
        }
 
        if (stream->sc->discarded_events_has_ts) {
+               /*
+                * Make the stream's state be in the time range of a
+                * discarded events message since we have the message's
+                * time range (`stream->sc->discarded_events_has_ts`).
+                */
                stream->discarded_events_state.in_range = true;
 
                /*
                 * The clock snapshot values will be validated when
-                * handling the next "packet beginning" message.
+                * handling the next packet beginning and end messages
+                * (next calls to handle_packet_beginning_msg() and
+                * handle_packet_end_msg()).
                 */
                cs = bt_message_discarded_events_borrow_beginning_default_clock_snapshot_const(
                        msg);
@@ -695,6 +773,11 @@ bt_self_component_status handle_discarded_events_msg(
 
        avail = bt_message_discarded_events_get_count(msg, &count);
        if (avail != BT_PROPERTY_AVAILABILITY_AVAILABLE) {
+               /*
+                * There's no specific count of discarded events: set it
+                * to 1 so that we know that we at least discarded
+                * something.
+                */
                count = 1;
        }
 
@@ -747,14 +830,24 @@ bt_self_component_status handle_discarded_packets_msg(
                goto end;
        }
 
+       /*
+        * Discarded packets messages are guaranteed to occur between
+        * packets.
+        */
        BT_ASSERT(!stream->packet_state.is_open);
 
        if (stream->sc->discarded_packets_has_ts) {
+               /*
+                * Make the stream's state be in the time range of a
+                * discarded packets message since we have the message's
+                * time range (`stream->sc->discarded_packets_has_ts`).
+                */
                stream->discarded_packets_state.in_range = true;
 
                /*
                 * The clock snapshot values will be validated when
-                * handling the next "packet beginning" message.
+                * handling the next packet beginning message (next call
+                * to handle_packet_beginning_msg()).
                 */
                cs = bt_message_discarded_packets_borrow_beginning_default_clock_snapshot_const(
                        msg);
@@ -770,6 +863,11 @@ bt_self_component_status handle_discarded_packets_msg(
 
        avail = bt_message_discarded_packets_get_count(msg, &count);
        if (avail != BT_PROPERTY_AVAILABILITY_AVAILABLE) {
+               /*
+                * There's no specific count of discarded packets: set
+                * it to 1 so that we know that we at least discarded
+                * something.
+                */
                count = 1;
        }
 
index a41052c2e6c04d39a00fe679166d2d08678fb7a2..c9b94ae3814485ea08f4f0e40e4f609e8583b733 100644 (file)
@@ -37,9 +37,24 @@ struct fs_sink_comp {
        /* Base output directory path */
        GString *output_dir_path;
 
+       /*
+        * True if the component assumes that it will only write a
+        * single CTF trace (which can contain one or more data
+        * streams). This makes the component write the stream files
+        * directly in the output directory (`output_dir_path` above).
+        */
        bool assume_single_trace;
+
+       /* True to completely ignore discarded events messages */
        bool ignore_discarded_events;
+
+       /* True to completely ignore discarded packets messages */
        bool ignore_discarded_packets;
+
+       /*
+        * True to make the component quiet (nothing printed to the
+        * standard output).
+        */
        bool quiet;
 
        /*
This page took 0.029481 seconds and 4 git commands to generate.