configure.ac, lib: rename "extra" (version) to "development stage"

"Extra" is a term which we'll use for something else brought by a
subsequent patch.

I took the "development stage" term from
<https://en.wikipedia.org/wiki/Software_release_life_cycle#Stages_of_development>,
where "Release candidate" is one of the stages.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I285fbf9851cde41a520079b4c31cdc5d8bf32412
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2835
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

lib: add bt_version_get_name() and bt_version_get_name_description()

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: If080c93994ac5869e29061b21d7b5c35387985d3
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2834
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

lib: bt_version_get_extra(): return `NULL` if none instead of empty str.

This follows the pattern we have for other optional strings returned by
the library.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I775f4f3be917bde405ad3b5e63183dae9609cf03
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2833
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

configure.ac: add version name/description definitions and report them

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: If287cce862facaaec71c63030ae578e24bcf4591
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2832
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-by: Michael Jeanson <mjeanson@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

Document libbabeltrace2's C API

This patch adds initial documentation for the Babeltrace 2 library's
C API using Doxygen.

The Doxygen project is located in `doc/api/libbabeltrace2`, as we can
eventually add `doc/api/libbabeltrace2-ctf-writer`.

To be able to use Doxygen's member grouping [1], I had to join some
header files (`const` and non `const` headers, for example), because
otherwise I could not get some functions in separate files to be in the
same member group in the order I want. In the end, the library user
includes `<babeltrace2/babeltrace.h>`, so how we organize the headers
exactly is not so crucial.

[1]: http://www.doxygen.nl/manual/grouping.html#memgroup

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I6d1dc2e7c5ee63fcd4220d0fd9f0931d361d2f31
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2807
Tested-by: jenkins <jenkins@lttng.org>

Fix: src.ctf.lttng-live: emitting stream end msg with no stream

Background
==========
When a stream hangs up on the `src.ctf.lttng-live` component, we make
sure we send a stream end message to ensure we honor The Contract which
states that any stream beginning must eventually be followed by its
stream end counterpart. We do this by calling
`ctf_msg_iter_get_next_message()` one last time to emit any missing
messages.

Using the upcoming lttng clear feature in conjunction with a per-pid
session makes it highly likely that a live stream hangs up on the
`src.ctf.lttng-live` component between the moment we learn about it and
the moment we first ask for its live index.

In such event, the live stream iterator and its `ctf_msg_it` are both
created but the corresponding stream is uninitialized.

When the component realized that a live stream has hung up, it calls
`ctf_msg_iter_get_next_message()` to respect The Contract but then
errors out here:
  CAUSED BY [lttng-live: 'source.ctf.lttng-live'] (msg-iter.c:2474)
    Cannot create stream end message because stream is NULL:
    msg-it-addr=0x555fba864010

The `stream` field is null because we never got the chance to received
any index for this stream.

Issue
=====
It's possible for a `ctf_msg` state machine to pass by the
`STATE_EMIT_MSG_STREAM_END` state without having passed by the
`STATE_EMIT_MSG_STREAM_BEGINNING` state.

Solution
========
Keep track of the fact that we sent a stream beginning message
downstream and that we need to send its respective stream end message.
If no message were send for a particular stream, we can omit sending a
stream end message.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: If7f52f43162e7263785713c01c226907fe475d94
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2719
CI-Build: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

lib: msg. iter. inactivity message has a simple CS, not a default CS

The "default clock snapshot" properties of some types of messages come
from the fact that a stream class has a default clock class, and
therefore its streams have a default clock.

A message iterator inactivity message is not related to any stream, so
it doesn't have a "default" clock class: it has a simple clock class,
and therefore a simple clock snapshot.

Update the C and Python APIs to show this.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I0142c4f91217791e3157d37a32f4e2f234afa8d2
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2801
Tested-by: jenkins <jenkins@lttng.org>

lib: remove self component param. from msg. iterator init. method

Since a3f0c7db ("lib: introduce bt_message_iterator_class"), the
`self_component` parameter of
`bt_message_iterator_class_initialize_method` is useless because you can
access the equivalent with bt_self_message_iterator_borrow_component().

Remove it.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I81e967acfd99b6ef3a2e01ae2ee19008a3c60408
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2761
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

lib: graph API: return borrowed references when adding to an object

Before this patch, the following functions return a new reference when
their last parameter is not `NULL`:

* bt_graph_add_filter_component()
* bt_graph_add_filter_component_with_initialize_method_data()
* bt_graph_add_simple_sink_component()
* bt_graph_add_sink_component()
* bt_graph_add_sink_component_with_initialize_method_data()
* bt_graph_add_source_component()
* bt_graph_add_source_component_with_initialize_method_data()
* bt_graph_connect_ports()
* bt_self_component_filter_add_input_port()
* bt_self_component_filter_add_output_port()
* bt_self_component_sink_add_input_port()
* bt_self_component_source_add_output_port()

I'm changing this so that they return a borrowed reference instead. This
is more in line with other non-creating functions which always return
borrowed references.

It's okay to borrow here because the object to which you add an object
becomes its owner anyway.

Most sites are updated by removing the *_put_ref() call as the reference
is now borrowed.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I71a5e18760504d8f8610162e3f6d7bd8d87474f9
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2762
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

lib: plugin-dev.h: rename `MESSAGE_ITERATOR` -> `MESSAGE_ITERATOR_CLASS`

This is more in line with the message iterator class concept and
indicates that a given method belongs to the (implicit) component
class's message iterator class.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Icbcefec886fcbb2b1928d4b1009f3aca88c032a0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2751
CI-Build: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

lib: rename "self comp. input port message iter." -> "message iterator"

This simplifies the terminology (and therefore the eventual
documentation). It's possible because we have a single type of message
iterator since 6c373cc9 ("lib: remove output port message iterator").

I just moved everything in
`self-component-port-input-message-iterator.h` to `message-iterator.h`
and removed the specific prefix. Header files are about to be reshaped
soon anyway with the C API documentation patch.

In the Python API, I changed *._create_input_port_message_iterator() to
*._create_message_iterator(). I didn't change
`_UserComponentInputPortMessageIterator` because it's not a public name,
so it's not critical to change it now.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I219b25495911363595bdf3b8b3f3b3cf802f20ac
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2749
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
CI-Build: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

lib: append `_FUNC` to `BT_PLUGIN_{INITIALIZE,FINALIZE}*`

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ie643815f2ec07149025d864324e6aefc55a14cd5
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2750
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

Replace `diamon.org/babeltrace` with `babeltrace.org`

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I87e6b0722358d74a0377b6b3f37ed81d65aeaa8c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2747

lib: create common base for bt_component_class_{source,filter}

There are multiple spots which deal with message iterators, that have
duplicated code for source and filter components.  The code is the same,
except that one side deals with a bt_component_class_source and the
other with a bt_component_class_filter.

This patch introduce a common base,
bt_component_class_with_iterator_class, that holds the message iterator
class property.  The aforementioned code paths can then be deduplicated.

Change-Id: Ib2b42da4e77a0ab7faf94533684a7c1d665eb2e9
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2744
Tested-by: jenkins <jenkins@lttng.org>

lib: introduce bt_message_iterator_class

Today, when defining a source or filter component class, the user sets
the message iterator methods directly on the class.  The `next`
methods is mandatory, so it is passed to
bt_component_class_{source,filter}_create, and the rest are optional, so
they are set by dedicated setters.  All these setters are therefore
duplicated for source and filter, for example:

  - bt_component_class_source_set_message_iterator_initialize_method
  - bt_component_class_filter_set_message_iterator_initialize_method

This patch factorizes everything related to message iterator methods and
introduces the concept of "message iterator class".  Instead of setting
the message iterator methods on a component class, the user will now
prepare a message iterator class, and pass a reference to this class
when creating a source or filter component class.  So, what used to be
this:

    src_cls = bt_component_class_source_create(my_iter_next_method);
    bt_component_class_source_set_message_iterator_initialize_method(my_iter_init_method);

would now become:

    iter_cls = bt_message_iterator_class_create(my_iter_next_method);
    bt_message_iterator_class_set_initialize_method(my_iter_init_method);
    src_cls = bt_component_class_source_create(iter_cls);

The message iterator class is a ref-counted object, and
bt_component_class_{source,filter}_create take their own references, so
a user would typically call bt_message_iterator_class_put_ref just after
that, as they would likely have no more use for the iterator class.  It
would be possible, in theory, to share an iterator class between
multiple component classes, but to this day no practical usage has been
found.  The search continues.

The macros to define a component class in a plugin remain the same,
where the message iterator methods are attached to the component class.
For example

    BT_PLUGIN_SOURCE_COMPONENT_CLASS_MESSAGE_ITERATOR_INITIALIZE_METHOD_WITH_ID

In other words, the message iterator class concept is not exposed in
this area.

There is a small change on the message iterator `initialize` methods
impacting existing components: the `initialize` method of
`bt_message_iterator_class` accepts a `bt_self_component` instead of a
specialized `bt_self_component_source` or `bt_self_component_filter`.
If the `initialize` method requires the specialized version, the
user should pass it through the user data attached to the component.
This had to be changed in the debug info component, for example.

Change-Id: Idf8666d028eadae34589cc0460dc1da19ca75765
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2725
Tested-by: jenkins <jenkins@lttng.org>

lib: run most of bt_self_component_port_input_message_iterator_try_finalize when iterator is in NON_INITIALIZED state

We hit a failed BT_ASSERT in the specific scenario explained below (and
illustrated by the new test).

The problem
-----------

Let's say we have a graph with this topology, where everything is
implemented in Python:

  MySink [in] <-> [out] MyFilter (MyFilterIter) [in] <-> [out] MySource (MySourceIter)

And the following sequence of events:

1. MySink's _user_graph_is_configured method creates an iterator on its
   "in" input port.  This runs MyFilterIter's __init__ method.
2. MyFilterIter's __init__ method starts by creating an upstream
   iterator on its "in" input port.  This iterator, of type
   MySourceIter, is initialized successfully.
3. MyFilterIter's __init__ method creates some other data structure which
   happens to form a Python reference cycle (the MyFilterIter instance has a
   reference on an object, which has a reference on the MyFilterIter
   instance).
4. MyFilterIter's __init__ method encounters an error, so an exception
   is raised.

When the MySourceIter is created, an entry is added to MyFilterIter's
upstream_msg_iters array (in the underlying
bt_self_component_port_input_message_iterator object).

When the exception is raised, because of the reference cycle, the
MyFilterIter Python object stays alive.  It has a reference on the
MySourceIter Python object, which keeps the underlying
bt_self_component_port_input_message_iterator object alive as well.

When the create_self_component_input_port_message_iterator call realizes
that the initialization of the filter iterator failed, it does a put_ref
on the iterator object, which ends up calling
bt_self_component_port_input_message_iterator_destroy.  In there, we
assert that:

  BT_ASSERT(iterator->upstream_msg_iters->len == 0);

This assertion does not hold, because the array still contains the entry
for the upstream iterator (on MySource).

Normally, the call to
bt_self_component_port_input_message_iterator_try_finalize, earlier in
bt_self_component_port_input_message_iterator_destroy, should take care
of unlinking any upstream or downstream iterator.  However, that step is
completely skipped because the iterator is still in the NON_INITIALIZED
state.

To reproduce, it is important to have the two conditions before the
exception is raised:

  - an upstream iterator is created: otherwise, the upstream_msg_iters
    array is empty so the assertion is true
  - a Python reference cycle exists, keeping the MyFilterIter object
    alive: otherwise, it is destroyed when the exception is raised (or
    caught?), which destroys the MySourceIter object, which destroys the
    underlying bt_self_component_port_input_message_iterator object (or
    the source iterator), which removes itself from the filter
    iterator's upstream_msg_iters array.  The array now being empty, the
    assertion would be true.

Note that the Python reference cycle is a user error that is not
desirable, but likely to happen.  To avoid any memory leak due to such a
reference cycle, I think the filter iterator here should normally make
sure to break the reference cycle.  In a successful execution, it would
be in _user_finalize.  If __init__ fails, it should make sure to not
leave a reference cycle in place, perhaps by using a try-except to clear
it before exiting the function.

The fix
-------

I believe it's wrong to skip the _try_finalize function even when we are
in this state, because it's possible (as shown above) for an iterator
initialize method to create some upstream iterators but then fail.  The
iterator remains in the NON_INITIALIZED state, but has some upstream
iterators.

I have changed
bt_self_component_port_input_message_iterator_try_finalize so that when
the iterator is in the NON_INITIALIZED, we do unlink the upstream and
downstream iterators.  However, I don't think we want to call the
iterator's finalize method in that case, since it hasn't been properly
initialized (a bit like a C++ object destructor is not called if the
constructor throws).  So I made it so the finalize method is only called
if the state is not NON_INITIALIZED.

Change-Id: Ibe2cbc729fc81ff0d68219400d925110242b7ae1
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2633
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

tests: plug memory leak in test_bin_info

When running test_bin_info (e.g. through
tests/plugins/flt.lttng-utils.debug-info/test_bin_info_i386-linux-gnu)
under Valgrind, I get:

==25792== 1,112 (88 direct, 1,024 indirect) bytes in 1 blocks are definitely lost in loss record 18 of 20
==25792==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25792==    by 0x55C2B10: g_malloc0 (gmem.c:129)
==25792==    by 0x55C86F2: g_option_context_new (goption.c:361)
==25792==    by 0x10CB25: main (test_bin_info.c:419)

Fix that by calling g_option_context_free.

Reported-by: Valgrind Memcheck
Change-Id: I9cd9a5ef786484169b9215744861af8cd6f5a9c8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2739
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

debug-info: free existing build-id in bin_info_set_build_id

When running test
tests/plugins/flt.lttng-utils.debug-info/test_bin_info_i386-linux-gnu, I
see:

Direct leak of 20 byte(s) in 1 object(s) allocated from:
    #0 0x7f623da26d38 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded38)
    #1 0x7f623ce37b10 in g_malloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
    #2 0x5583ef04ad8f in test_bin_info_build_id /home/smarchi/src/babeltrace/tests/plugins/flt.lttng-utils.debug-info/test_bin_info.c:239
    #3 0x5583ef04bd01 in main /home/smarchi/src/babeltrace/tests/plugins/flt.lttng-utils.debug-info/test_bin_info.c:445
    #4 0x7f623c7f7b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

This is because a build id is set twice on the same bin_info object in
test_bin_info_build_id.  However, bin_info_set_build_id doesn't free the
existing build id, if there is one, before assigning the new build id.

Fix that by freeing the existing build id, if any, before allocating the
new one.

Reported-by: ASan
Change-Id: I66409294bf11accde6c9d54a5e07572f9a995ff6
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2738
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

cli: free log level string value

In bt_config_convert_from_args, when encoutering a --log-level argument
applied to a non-option argument, we create a string bt_value.  We add
it to an array, which takes its own reference on it.  However, in the
successful case, we never drop our reference to it, so it's never freed.
Fix it by calling bt_value_put_ref on it in all cases.

This is the error reported by address sanitizer:

Direct leak of 128 byte(s) in 2 object(s) allocated from:
    #0 0x7f1e36724d38 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded38)
    #1 0x7f1e35d97b10 in g_malloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
    #2 0x558c39c19d3b in bt_config_convert_from_args /home/smarchi/src/babeltrace/src/cli/babeltrace2-cfg-cli-args.c:3445
    #3 0x558c39c1f9e4 in bt_config_cli_args_create /home/smarchi/src/babeltrace/src/cli/babeltrace2-cfg-cli-args.c:4654
    #4 0x558c39c233db in bt_config_cli_args_create_with_default /home/smarchi/src/babeltrace/src/cli/babeltrace2-cfg-cli-args-default.c:74
    #5 0x558c39c0781a in main /home/smarchi/src/babeltrace/src/cli/babeltrace2.c:2647
    #6 0x7f1e35757b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Reported-by: ASan
Change-Id: Ib2251d9dd8628bda128ae4d2e756d0d86fa12163
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2737
Tested-by: jenkins <jenkins@lttng.org>

bt2: free port user data when finalizing components

When creating component ports in Python, it is possible to pass a Python
object as user data:

    class MySource(
        bt2._UserSourceComponent, message_iterator_class=MyIter
    ):
        def __init__(self, config, params, obj):
            self._add_output_port('out', MyUserData())

The port takes a reference to this Python object, thanks to the `void *`
typemap in native_bt_port.i:

    %typemap(out) void * {
            Py_INCREF($1);
            $result = $1;
    }

However, this reference is never released.

This patches makes it so that when a component is finalized, it releases
the reference for the user data of all of its ports.

Change-Id: Ifebecc3b242c2ccf2bd65347a9087b90093f286c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2734
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

build: try calling python-config with --embed

To get the flags required to link an application with Python 3.8 (such
as to embed Python in the application), it is necessary to use the
"--embed" flag of python-config.  These flags include "-lpython3.8".
Without "--embed", the printed flags are for building a Python
extension.  These don't require being linked with the Python library,
since they are dlopen'ed by the library itself.

Our Python plugin provider requires being linked with Python, since it
embeds a Python interpreter.  Since we don't use "--embed" currently
when getting link flags, we don't link with the Python library, and
therefore the provider is not usable with Python 3.8.

The solution proposed here:

  https://docs.python.org/3/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build

is to try calling python-config with --embed first, and then without if
that didn't work.  With this patch, I am able to use the Python plugin
provider with Python 3.8.

Change-Id: I0c0e61dd3bded853abf124c651efe98ee7700101
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2726
Reviewed-by: Michael Jeanson <mjeanson@efficios.com>

tests: remove unnecessary message iterator classes

These user message iterator classes don't do anything special, we can
remove them and pass bt2._UserMessageIterator directly to the components
instead, reducing a little bit the noise in the tests.

Some classes in test_component_class.py were simply unused, so I removed
them.

Change-Id: I79f787b9d121a3dc6456f81090ebf51d088d2c73
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2736
Tested-by: jenkins <jenkins@lttng.org>

tests: make test_sink_self_port_user_data actually test a sink

This test method is meant to test a sink, but currently tests a filter,
fix that.

Change-Id: Icca321f50a43b709b64f15c885c37c6e7106653d
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2735
Tested-by: jenkins <jenkins@lttng.org>

lib: remove unnecessary (void *) cast in extend_map_element

Change-Id: I238aeef707d2822030c18e2a8dc1ebf4d432a9f2
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2732
Tested-by: jenkins <jenkins@lttng.org>

cli: fix bt_plugin leak when using `-i ctf`

When running:

    libtool --mode=execute valgrind --leak-check=full  ./src/cli/babeltrace2 /home/smarchi/src/babeltrace/tests/data/ctf-traces/succeed/succeed1 -i ctf

I get:

    2,680 (168 direct, 2,512 indirect) bytes in 1 blocks are definitely lost in loss record 56 of 58
       at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
       by 0x5360B10: g_malloc0 (gmem.c:129)
       by 0x4E97502: bt_plugin_create_empty (plugin.h:178)
       by 0x4E9AD72: bt_plugin_so_create_empty (plugin-so.c:1239)
       by 0x4E9B0D1: bt_plugin_so_create_all_from_sections (plugin-so.c:1342)
       by 0x4E9BFBF: bt_plugin_so_create_all_from_file (plugin-so.c:1678)
       by 0x4E93FB4: bt_plugin_find_all_from_file (plugin.c:210)
       by 0x4E95300: nftw_append_all_from_dir (plugin.c:551)
       by 0x5956F13: process_entry (ftw.c:464)
       by 0x59573BA: ftw_dir (ftw.c:543)
       by 0x5957BEB: ftw_startup (ftw.c:768)
       by 0x4E95748: bt_plugin_create_append_all_from_dir (plugin.c:641)

The call to find_loaded_plugin at babeltrace2-cfg-cli-args.c:4013
returns a new reference to the plugin, for which we don't have a
corresponding put_ref.

Looking at all the users of find_loaded_plugin, they only really need to
borrow the plugin, so rather than add a put_ref, I've made it so
find_loaded_plugin returns a borrowed reference instead of a new
reference.  For clarity, I've renamed that function to
borrow_loaded_plugin_by_name.  And for consistency, I've renamed
borrow_loaded_plugin to borrow_loaded_plugin_by_index.

Change-Id: I19234bda6e219efa3e55da760846d138c381fac4
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reported-by: Valgrind Memcheck
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2731
Tested-by: jenkins <jenkins@lttng.org>

cli: remove unused structures and enums

These have apparently been unused for a while, since:

    commit db0f160afd671de44e52d2b364de957ddccdac02
    Author: Philippe Proulx <eeppeliteloop@gmail.com>
    Date:   Fri Mar 3 00:13:36 2017 -0500

        CLI: add `run` command and make `convert` command use it

Change-Id: Ib8ce061540a0c268d3949a565c570142f37e123c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2728
Tested-by: jenkins <jenkins@lttng.org>

tests: fix test failure with msys2's Python 3.8.1-1 package

msys2's Python package version 3.8.1-1 produces wrong output for
PureWindowsPath's string representation.  It does this:

    >>> import pathlib
    >>> str(pathlib.PureWindowsPath('/yo/madame'))
    '/yo/madame'

When it should do this:

    >>> import pathlib
    >>> str(pathlib.PureWindowsPath('/yo/madame'))
    '\\yo\\madame'

Because of this, tests/plugins/src.ctf.fs/query/test_query_trace_info.py
is currently failing, as the Babeltrace output contains back-slashes but
the regex we produce contains forward-slashes.

The issue appears to be fixed in 3.8.1-2:

    https://github.com/msys2/MINGW-packages/commit/7ce8394ec8af3bdef83d1a24fd9a96bf8da3c154#diff-8b71128fa8f1e4e070196eeb2fc9a19d

But anyway, it's not really necessary to use PureWindowsPath and
PurePosixPath.  I changed the code to just use a version of the regex
with back-slashes when on Windows, and with front-slashes when on the
others.

Change-Id: Idcd865d87350682644a536ada95cfac161cc1182
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2733
Tested-by: jenkins <jenkins@lttng.org>

trimmer: free GMatchInfo object in set_bound_from_str

When a call to compile_and_match succeeds, it returns an allocated
GMatchInfo object in `*match_info`, which is never freed, causing a
memory leak:

    ==3711000== 508 (72 direct, 436 indirect) bytes in 1 blocks are definitely lost in loss record 33 of 42
    ==3711000==    at 0x483AB65: calloc (vg_replace_malloc.c:762)
    ==3711000==    by 0x49CB7C1: g_malloc0 (in /usr/lib/libglib-2.0.so.0.6200.4)
    ==3711000==    by 0x49BCEAA: ??? (in /usr/lib/libglib-2.0.so.0.6200.4)
    ==3711000==    by 0x49BD310: g_regex_match_full (in /usr/lib/libglib-2.0.so.0.6200.4)
    ==3711000==    by 0x49BDEEA: g_regex_match (in /usr/lib/libglib-2.0.so.0.6200.4)
    ==3711000==    by 0x510CF25: compile_and_match (trimmer.c:187)
    ==3711000==    by 0x510D55D: set_bound_from_str (trimmer.c:378)
    ==3711000==    by 0x510D90A: set_bound_from_param (trimmer.c:463)
    ==3711000==    by 0x510DEBE: init_trimmer_comp_from_params (trimmer.c:568)
    ==3711000==    by 0x510E090: trimmer_init (trimmer.c:643)
    ==3711000==    by 0x4886457: add_component_with_init_method_data (graph.c:971)
    ==3711000==    by 0x4886C28: bt_graph_add_filter_component_with_initialize_method_data (graph.c:1075)

Fix that by calling g_match_info_free at the end of the function.

Reported-by: Valgrind Memcheck
Change-Id: Iee42f63d45f5761e1191dcc6d3f6d47e75a4123c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2727
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

bt2: rename `object` parameter -> `object_name`

I think it's more evident this way.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I20f80ab8b28c4f4f0d390dd9fb4676ff69e8e609
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2712
Tested-by: jenkins <jenkins@lttng.org>

lib: remove bt_query_executor_interrupt, add bt_query_executor_borrow_default_interrupter

It is currently possible to interrupt a running quer executor with
bt_query_executor_interrupt.  It is however not possible to reset the
default interrupter that this function sets and resume the query
execution.

Rather than add a new query executor function to reset the default query
executor interrupter, introduce a new function,
bt_query_executor_borrow_default_interrupter, to borrow that default
interrupter.  All the bt_interrupter API is therefore accessible with
this default interrupter.

This patch removes the bt_query_executor_interrupt function, since it's
no longer needed.

Change-Id: I877dfbf22e36233750a71220fc5ab0297f8c0c28
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2709
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

lib: remove bt_graph_interrupt, add bt_graph_borrow_default_interrupter

It is currently possible to interrupt a running graph with
bt_graph_interrupt.  It is however not possible to reset the default
interrupter that this function sets and resume the graph execution.

Rather than add a new graph function to reset the default graph
interrupter, introduce a new function,
bt_graph_borrow_default_interrupter, to borrow that default interrupter.
All the bt_interrupter API is therefore accessible with this default
interrupter.

This patch removes the bt_graph_interrupt function, since it's no longer
needed.

Change-Id: I277e6c8bb4e1be0a6557a6287b7ba8997e20d27b
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2708
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

lib: graph API: remove "listener removed" callback parameters

Before this patch, when you add a "port added" listener to a graph,
you can pass a callback which gets called when the listener is removed.
This only happens when the graph is destroyed.

This "listener removed" callback feature seems unnecessary as the graph
user, who adds the "port added" listener, has total control over the
graph object. Therefore she can ensure that anything needed by her "port
added" listeners exists as long as the graph exists.

Therefore this patch removes those parameters and everything related.

In Python, we used to keep a strong reference on the partial Python
object (listener's data) and release it when our "listener removed"
function was called. Now, the listener's data is a weak reference, but
we keep a list of strong partial references within the graph object
itself (`self._listener_partials`) to ensure that the partials exist as
long as the graph exists.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I4c06ff139740f887ae2ace7633d2edeb01fd2fa0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2637
Tested-by: jenkins <jenkins@lttng.org>

lib, bt2: graph API: remove "ports connected" listeners

Two ports being connected is always the consequence of the graph user
calling bt_graph_connect_ports() and this function returning
`BT_GRAPH_CONNECT_PORTS_STATUS_OK`. In other words, this event cannot
occur without a direct, concomitant action by the graph user.

Knowing this, the "ports connected" graph listeners are useless.

The "port added" listeners remain useful: a component can add a port at
many moments during the graph configuration phase, therefore having a
"port added" listener can avoid checking if the current graph components
have new ports every time you call bt_graph_add_*_component*() or
bt_graph_connect_ports().

This patch removes everything related to the "ports connected" graph
listeners.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I218c7b7b57c52f2e8589b35e3d89f38dfd961c0a
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2636
Tested-by: jenkins <jenkins@lttng.org>

babeltrace2-plugin-ctf(7): "theirs" -> "its" (single CTF trace)

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I251e8c5c2a357637983be92abb597827ffadd7da
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2640

.gitignore: add missing `/tests/lib/test_remove_..._destruction_listener`

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I20a520668a9f2bff7c47891072f907fd2b53d9e6
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2635
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

Sync argpar with upstream

Sync with commit:

    92ecd98e487729d7ec268a986390b624fc394feb
    Add missing va_end in argpar_vasprintf

Change-Id: If005cbc043af96e4db9ad4d1a8b2d341944383d0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2624

Use argpar from upstream

Now that argpar is maintained in a separate repository, sync the code
with it and remove tests from this code base.

The code was sync'ed  with the argpar repository at commit:

    f46b510674785c70781a3de06c02888faded5db9 (HEAD -> master,
    Strip trailing spaces

Change-Id: Ie805db224f2d3890decbed26a1c80c105d083293
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2616

bt2: use format_bt_error and format_bt_error_cause to generate _Error and _ErrorCause string representations

This makes it so that formatting bt2._Error as a string looks the same
way as what is printed by the CLI.  It also allows formatting of error
causes individually.

The __str__ methods generate string representations that do not include
terminal color control characters. If we want, we could later add a
"to_string" method that can optionally generate a string with terminal
color control characters.

Change-Id: I9e2c6db536a1bea46b07c5fe8bb13f702d8accce
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2435
Tested-by: jenkins <jenkins@lttng.org>

string-format: introduce function to format a bt_error_cause

In the Python bindings, we'll want to be able to format single error
causes.  Factor out the code to format one bt_error_cause from
format_bt_error.

Change-Id: Ibf5b0363e9a239be6228580246b6613743ace09e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2434
Tested-by: jenkins <jenkins@lttng.org>

string-format: introduce function to format a bt_error

The CLI has a function to format a bt_error nicely.  We will want to use
it in the Python bindings too, so move it to the string-format
convenience library.  Make it return the formatted string instead of
printing directly to stderr.

I did not try to make this function handle memory allocation errors, as:

1. It's not very likely anyway
2. We are likely in the process of handling an error, I don't think it
   would be useful to return an error code.  If something fails in this
   function, I don't know what else remains to do.

Change-Id: Ia540b95bd9e1aca7899e5fbccfe3fba463457e3c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2433
Tested-by: jenkins <jenkins@lttng.org>

string-format: introduce function to format component class name

The CLI has a function to print a component class name nicely,
print_plugin_comp_cls_opt.  The same functionality will be needed by
the Python bindings, move the function to a new convenience library.
Modify the function to return a string instead of printing directly to a
stream.  Update users accordingly.

Change-Id: I83fb61107ce93497ed34ebc383cbd3185c19752c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2432
Tested-by: jenkins <jenkins@lttng.org>

common: introduce struct bt_common_color_codes and function bt_common_color_get_codes

The behavior of the color functions is currently initialized at startup.
If we detect that colors should be enabled, we set the
bt_common_color_code_* variables once and for all.

The following patches will introduce string formatting functions that
optionally use color, based on a parameter with three values:

 - always: always use colors, regardless of what we detected at startup
 - never: never use colors, regardless of what we detected at startup
 - auto: use colors based on what we detected at startup

One option would be to add some color functions that take a parameter,
so you could do:

  bt_common_color_bg_cyan_opt(BT_COMMON_COLOR_WHEN_ALWAYS)
  bt_common_color_bg_cyan_opt(BT_COMMON_COLOR_WHEN_NEVER)
  bt_common_color_bg_cyan_opt(BT_COMMON_COLOR_WHEN_AUTO)

However, I find this very verbose, so not very practical.

This patch introduces a new structure type bt_common_color_codes that
holds the color codes for the current terminal, or empty strings.  It
also introduces a function bt_common_color_get_codes that accepts an
always/never/auto parameter.  It initializes a bt_common_color_codes
structure with the color codes (or empty strings) to use.

This will allow the string formatting function to conditionally use
colors without being too verbose:

  struct bt_common_color_codes codes;
  bt_common_color_get_codes(&codes, use_colors);
  printf("%sHello%s\n", codes.fg_cyan, codes.reset);

Change-Id: I887e736291e9e1fe54156a81ce4011b3906d6f3c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2431
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

lib: standardize variant field option function names

This patch does the following changes:

bt_field_variant_select_option_field_by_index():
    bt_field_variant_select_option_by_index()

bt_field_variant_get_selected_option_field_index():
    bt_field_variant_get_selected_option_index()

bt_field_variant_borrow_selected_class_option_const():
    bt_field_variant_borrow_selected_option_class_const()

bt_field_variant_with_unsigned_integer_selector_borrow_selected_class_option_const():
    bt_field_variant_with_selector_field_integer_unsigned_borrow_selected_option_class_const()

bt_field_variant_with_signed_integer_selector_borrow_selected_class_option_const():
    bt_field_variant_with_selector_field_integer_signed_borrow_selected_option_class_const()

A variant field is the instance of a variant field class. A variant
field has one or more options which are instances of the variant field
class options. A variant field option contains a field.

The `with_unsigned_integer_selector` to
`with_selector_field_integer_unsigned` and
`with_signed_integer_selector` to `with_selector_field_integer_signed`
renames match what's already in the `bt_field_class` API.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I7c8f19d06d86464b0b2131036c27adb8d909aaa3
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2571
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

bt2: don't print previous causes in causes created from bt2._Error

When an API call from Python fails, we create a bt2._Error that will
take the bt_error from the current thread and assume its ownership.  If
that bt2._Error escapes the Python code and is caught by the Babeltrace
native code, we convert that bt2._Error to a new cause.  We call str()
on the bt2._Error (through bt_py_common_format_exception) to obtain the
text with which to create the new cause.  However, the str() for
bt2._Error textually formats all the existing causes.  As a result, the
text for the new cause includes the string formatting of all the
previous causes.  When the CLI, for example, ends up printing all error
causes successively, it becomes very confusing: the text of one cause
includes the text of previous causes.

What we want, in fact, is not an str() of the bt2._Error, but just a
message that includes the traceback (to know where in the Python code
the cause was created) and the message passed to bt2._Error.__init__.
The traceback is just a goodie: we don't include it for causes created
in C, but since it's easy to obtain in Python it is nice to have.  There
is also the fact the file:line information for causes created in Python
is bogus (which should be fixed at some point), so the traceback fills
in for that for the moment.

This patch changes
restore_current_thread_error_and_append_exception_chain_recursive to
do this for bt2._Error.  For other exceptions, we still call
bt_py_common_format_exception, which ends up calling their __str__.

Here's an example:

    Traceback (most recent call last):
      File "test_simple.py", line 32, in <module>
        graph.run()
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/graph.py", line 188, in run
        utils._handle_func_status(status, 'graph object stopped running')
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/utils.py", line 140, in _handle_func_status
        raise bt2._Error(msg)
    bt2.error._Error: graph object stopped running
    [libbabeltrace2] (/home/smarchi/src/babeltrace/src/lib/graph/graph.c:600)
    Component's "consume" method failed: status=ERROR, comp-addr=0x1f15f50, comp-name="snk", comp-log-level=NONE, comp-class-type=SINK, comp-class-name="DummySink", comp-class-partial-descr="", comp-class-is-frozen=1, comp-input-port-count=1, comp-output-port-count=0
    [snk: sink.DummySink] (/home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/native_bt_log_and_append_error.h:99)
    Traceback (most recent call last):
      File "test_simple.py", line 26, in _user_consume
        next(self._iter)
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/message_iterator.py", line 58, in __next__
        status, 'unexpected error: cannot advance the message iterator'
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/utils.py", line 140, in _handle_func_status
        raise bt2._Error(msg)
    bt2.error._Error: unexpected error: cannot advance the message iterator
    [libbabeltrace2] (/home/smarchi/src/babeltrace/src/lib/graph/iterator.c:928)
    Component input port message iterator's "next" method failed: iter-addr=0x1ec1980, iter-upstream-comp-name="src", iter-upstream-comp-log-level=NONE, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="SourceWithFailingIter", iter-upstream-comp-class-partial-descr="", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
    [src (out): src.SourceWithFailingIter] (/home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/native_bt_log_and_append_error.h:102)
    Traceback (most recent call last):
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/message_iterator.py", line 201, in _bt_next_from_native
        msg = next(self)
      File "test_simple.py", line 8, in __next__
        raise ValueError('User message iterator is failing')
    ValueError: User message iterator is failing

    [libbabeltrace2] (/home/smarchi/src/babeltrace/src/lib/graph/iterator.c:928)
    Component input port message iterator's "next" method failed: iter-addr=0x1ec1980, iter-upstream-comp-name="src", iter-upstream-comp-log-level=NONE, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="SourceWithFailingIter", iter-upstream-comp-class-partial-descr="", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
    [src (out): src.SourceWithFailingIter] (/home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/native_bt_log_and_append_error.h:102)
    Traceback (most recent call last):
      File "/home/smarchi/build/babeltrace/src/bindings/python/bt2/build/build_lib/bt2/message_iterator.py", line 201, in _bt_next_from_native
        msg = next(self)
      File "test_simple.py", line 8, in __next__
        raise ValueError('User message iterator is failing')
    ValueError: User message iterator is failing

Notice how the "ValueError: User message iterator is failing" cause
seems to appear twice?  The first of them is in fact part of the text of
the "bt2.error._Error: unexpected error: cannot advance the message
iterator" cause.  With this patch applied, the first one of these does
not appear.

Change-Id: I9b3e2e6f78f5ee2ba49a0956f6ef77dbb23dadc4
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2557
Tested-by: jenkins <jenkins@lttng.org>

bt2: reverse order of printed causes in _Error.__str__

To match how the CLI prints them.

Change-Id: Ief913b119b169bf9f620531f8763db93e19bda27
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2556
CI-Build: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

bt2: remove ptr parameter of _Error.__init__

It isn't used.  We never pass a bt_error pointer when constructing an
_Error, the constructor of _Error always takes the bt_error from the
current thread.

Change-Id: I3c5920afe217f3b2067f9fb397b8ef8069d71b11
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2555
Tested-by: jenkins <jenkins@lttng.org>

tests: add test for list-plugins CLI command

The test provides a custom Python plugin, then checks for a
corresponding entry in the "list-plugins" output.

Change-Id: I65fff2d4d22f21a89fa1ad7e747c5e15677212b9
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2421
Tested-by: jenkins <jenkins@lttng.org>

tests: test removing a destruction listener from a destruction listener

This patch adds a test to verify that removing a trace or trace class
destruction listener from an object from within a destruction listener
of that same object works correctly.

It tests 3 scenarions:

- A destruction listener removing itself.
- A destruction listener removing a destruction listener that was
  already called.
- A destruction listener removing a destruction listener that is not yet
  called.

This assumes that destruction listeners are called in the order in which
there were added.

In the third scenario (removing a destruction listener that is not yet
called), the result is that the removed listener won't get called.

Change-Id: I49de9b662b3c1f77ca1c9f84c2c3575a8616cc10
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2429
Tested-by: jenkins <jenkins@lttng.org>

tests/lib/Makefile.am: Move libbabeltrace2-common and libbabeltrace2-logging to COMMON_TEST_LDADD

These libraries are necessary to all tests that use BT_ASSERT, which is
the case of pretty much every test.

Change-Id: Ifdc7e21f7540c694533f15f3fd55f46738683464
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2428
Tested-by: jenkins <jenkins@lttng.org>

bt2: make _ListenerHandle not hold a strong reference on the target object

The bt2._ListenerHandle object currently holds a strong reference to the
Python object on which the listener was added.  This allows validating
that a handle passed to the remove_destruction_listener method of an
object mathces that object.

However, keeping that strong reference also prevents the destruction of
that target object.  So, adding a destruction listener and keeping the
handle around actually prevents the destruction from happening, making
that destruction listener useless.

Change it so the _ListenerHandle only keeps the address of the target
object.  This is enough to do the check described above.  We must
however invalidate the _ListenerHandle when the target object is
destroyed, because another object could be later created at the same
address.  To achieve this, the handle object is bound to the destruction
callback, so that we can invalidate it in
_trace_destruction_listener_from_native /
_trace_class_destruction_listener_from_native.

The "del" statements in the tests were necessary before, otherwise the
handles would keep the trace class / trace alive, and the destruction
listeners would not get called.  I removed them, so it now tests that
keeping a listener handle doesn't keep the target object alive.

Change-Id: I668cf29b5a6046a89d4eff73d322cb0cd83e5109
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2426
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

bt2: fix error message in trace_class.py

This error message was probably copied from trace.py.  It should say
"trace class" and "trace".

Change-Id: Ib9fcae27102a90ffe5492e74906ca1a1d5205dfc
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2425

bt2: make Graph add listener methods return None

The methods add_ports_connected_listener and add_port_added_listener of
bt2.Graph currently return a _ListenerHandle object.  This is not useful
today, since it's not possible to remove those listeners (unlike trace
class and trace destruction listeners).

Make them not return a handle for now.  It will be possible to make them
return a handle again if/when we add the possibility to remove those
listeners.

Change-Id: I5ea1c9d8793232c2f7077b5c2b82797438066d13
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2424
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

param-parse: allow duplicate map keys

It is a bit odd that this is accepted (the second value overwrites the
first):

  -p a=3 -p a=4

but this isn't:

  -p a=3,a=4

Remove the check that prevents duplicate keys from being defined in a
map.

Change-Id: Id39e86bb48ac52c6714db3128b7d767f5347de64
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2423

bt2: validate parameters to _StreamClass.create_event_class before creating the native object

For the same reasons outlined in d3bf1370a437 ("bt2: validate parameters
to _TraceClass.create_stream_class before creating the native object"),
validate the parameters when creating an event class before creating the
native object.

Change-Id: I435b21c145e663a373f3e989feac3e1d5de626c8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2415

bt2: add invalid parameter type test for _UserComponent._create_trace_class

Verify that passing a value of the wrong type to _UserComponent._create_trace_class
produces an error.

Change-Id: Id8c0c72ff0a44b292b56095e8dcc1305b73a73a7
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2414

src.ctf.fs: sort inputs paths

Sort input paths to make the behavior of the component more
deterministic.

For example, when there are duplicated packets in the trace and we pick
one copy, this will ensure that we always process the data stream files in
the same order, and that we always pick the same copy of the packet.

In practice, this will reduce the chances of observing different
behaviors on different platforms where glib reports the files of a trace
in different orders.

A test is added to verify that the choice of which copy of a packet to
choose, in case a packet is present in multiple copies, is independent
from the order of the inputs paths passed to the component.

Change-Id: I034ba8c909e76a1e89589e1f0e0f37512c9d88bf
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2396

cli: print error causes in all error paths

I noticed that providing a parameter with a wrong syntax to a query
resulted in no error stack shown by the CLI, even though a bunch of
BT_CLI_LOGE_APPEND_CAUSE are done along the way.  We just get the logged
errors:

    ./src/cli/babeltrace2 query -p 'a=2,' src.ctf.fs yo
    11-15 16:17:34.734  6423  6423 E CLI/CFG-CLI-ARGS bt_config_query_from_args@babeltrace2-cfg-cli-args.c:1530 Invalid format for --params option's argument:
        Expecting unquoted map key:

        a=2,
            ^

    11-15 16:17:34.735  6423  6423 E CLI main@babeltrace2.c:2781 Command-line error: retcode=1

This is because the main function only prints the error stack when the
command execution fails, not when anything earlier fails, like the
creation of the command configuration in this case.

Move the call to print_error_causes at the end, so that the causes are
printed every time we exit with status code 1.

I then found that the amount of vertical blank space in the result was
too damn high:

    myprompt$ ./src/cli/babeltrace2 query -p 'a=2,' ctf src.ctf.fs yo
    11-15 16:38:15.850 13799 13799 E CLI/CFG-CLI-ARGS bt_config_query_from_args@babeltrace2-cfg-cli-args.c:1530 Invalid format for --params option's argument:
        Expecting unquoted map key:

        a=2,
            ^

    11-15 16:38:15.851 13799 13799 E CLI main@babeltrace2.c:2781 Command-line error: retcode=1

    ERROR:    [Babeltrace CLI] (/home/smarchi/src/babeltrace/src/cli/babeltrace2.c:2781)
      Command-line error: retcode=1
    CAUSED BY [Babeltrace CLI] (/home/smarchi/src/babeltrace/src/cli/babeltrace2-cfg-cli-args.c:1530)
      Invalid format for --params option's argument:
          Expecting unquoted map key:

          a=2,
              ^

    myprompt$

So I removed the newlines added by ini_append_error_expecting.  I think
it looks readable and more concise this way.  Any caller who wants the
newlines can add them itself.

    myprompt$ ./src/cli/babeltrace2 query -p 'a=2,' ctf src.ctf.fs yo
    11-15 16:41:38.600 15717 15717 E CLI/CFG-CLI-ARGS bt_config_query_from_args@babeltrace2-cfg-cli-args.c:1530 Invalid format for --params option's argument:
        Expecting unquoted map key:

        a=2,
            ^
    11-15 16:41:38.600 15717 15717 E CLI main@babeltrace2.c:2781 Command-line error: retcode=1

    ERROR:    [Babeltrace CLI] (/home/smarchi/src/babeltrace/src/cli/babeltrace2.c:2781)
      Command-line error: retcode=1
    CAUSED BY [Babeltrace CLI] (/home/smarchi/src/babeltrace/src/cli/babeltrace2-cfg-cli-args.c:1530)
      Invalid format for --params option's argument:
          Expecting unquoted map key:

          a=2,
              ^
    myprompt$

Change-Id: Id38159896d595b9c8fcac00b3364577e1ea36883
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2394
Tested-by: jenkins <jenkins@lttng.org>

tests: add missing backslash in tests/Makefile.am

Change-Id: I676829a841ad09e6e2df9894272f6c85a9cff1ca
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2420
Tested-by: jenkins <jenkins@lttng.org>

tests: test_auto_source_discovery_grouping: remove dir_sep variable

It isn't used.  Fixes the following shellcheck diagnostics:

    In test_auto_source_discovery_grouping line 42:
            dir_sep='\'
                     ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.

    In test_auto_source_discovery_grouping line 44:
            dir_sep='/'
            ^-----^ SC2034: dir_sep appears unused. Verify use (or export if used externally).

Reported-by: shellcheck
Change-Id: I9db00870cd697d5ad3bd5aa362a030a4b75f4e4c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2419
Tested-by: jenkins <jenkins@lttng.org>

tests: silence "variable/expression in single quote" shellcheck warnings

shellcheck complains that we use a variable in single quotes:

    In test_trace_copy line 63:
                    uniq_ts_cnt="$("${BT_TESTS_AWK_BIN}" '{ print $1 }' < "${text_output1}" | sort | uniq | wc -l)"
                                                         ^------------^ SC2016: Expressions don't expand in single quotes, use double quotes for that.

In other cases, it's because of backticks (`) in a single quote string.

Silence these warnings locally by putting the appropriate comment:

    # shellcheck disable=SC2016

Reported-by: shellcheck
Change-Id: Ib2cfa73f84ba8746d3793e49721ead171c24dd99
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2418
Tested-by: jenkins <jenkins@lttng.org>

tests: quote ${BT_CTF_TRACES_PATH} in test_trace_read and test_trace_copy

Fixes these shellcheck errors:

    In test_trace_copy line 33:
    SUCCESS_TRACES=(${BT_CTF_TRACES_PATH}/succeed/*)
                    ^-------------------^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

    In test_trace_read line 29:
    SUCCESS_TRACES=(${BT_CTF_TRACES_PATH}/succeed/*)
                    ^-------------------^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

    In test_trace_read line 30:
    FAIL_TRACES=(${BT_CTF_TRACES_PATH}/fail/*)
                 ^-------------------^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

Reported-by: shellcheck
Change-Id: Ia13810be69d2bc57f6d6541887de0f9d75e3ffe5
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2417
Tested-by: jenkins <jenkins@lttng.org>

tests: declare and assign variables separately in test_exit_status

Fixes these shellcheck errors:

    In test_exit_status line 34:
            local actual_stdout=$(mktemp -t test_cli_exit_status_stdout_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

    In test_exit_status line 35:
            local actual_stderr=$(mktemp -t test_cli_exit_status_stderr_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

    In test_exit_status line 53:
            local actual_stdout=$(mktemp -t test_cli_exit_status_stdout_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

    In test_exit_status line 54:
            local actual_stderr=$(mktemp -t test_cli_exit_status_stderr_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

    In test_exit_status line 72:
            local actual_stdout=$(mktemp -t test_cli_exit_status_stdout_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

    In test_exit_status line 73:
            local actual_stderr=$(mktemp -t test_cli_exit_status_stderr_actual.XXXXXX)
                  ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

Reported-by: shellcheck
Change-Id: Iddb1c4e8b0a3bc1c37cc08a37b1103d97b8560e7
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2416
Tested-by: jenkins <jenkins@lttng.org>

Make some bt_param_validation_map_value_entry_descr variables static

These variables are not used outside their respective files.

Change-Id: I281df2f935ebd4326a9a655cd8b84fbcb437cfc3
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2412
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

src.ctf.fs: make ctf_fs_ds_group_medops symbol hidden

Change-Id: I96c7f3aaf76c94337cd0efb1eb71b9cf76a1e5fa
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2411
Tested-by: jenkins <jenkins@lttng.org>

ctf: make ctf scanner symbols hidden

Change-Id: I21d2e25fe96d308ae949d2e3ac090e05cabbefaf
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2410
Tested-by: jenkins <jenkins@lttng.org>

param-validation: make symbols hidden

This convenience library is linked statically with the various plugins
that use it.  The symbols are currently not marked as hidden, which
causes the plugins that use it (e.g. babeltrace-plugin-ctf.so) to expose
them publically.  Sad.

Mark the symbols as hidden to avoid that.

Change-Id: If099b529f7c126f5defd5242486009c3fd4195eb
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2409
Tested-by: jenkins <jenkins@lttng.org>

python-plugin-provider: make python_state static

This variable is not used outside the file, it should not be exposed.

Change-Id: Icb1ed68485d9205149a2adf95051f4694cbc8cc0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2408
Tested-by: jenkins <jenkins@lttng.org>

lib: add comments to exposed but internal symbols

These two functions, despite being exposed by the shared library, are
not part of the public ABI.  They are only meant to be used by the
Python plugin provider, which is conceptually part of libbabeltrace2,
but implemented as a separate shared library.  Add comments to explain
that.

Change-Id: I704240070403c9dd5d75f15e107c00b011d8a5ef
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2407
Tested-by: jenkins <jenkins@lttng.org>

lib: add comment to bt_plugin_so_create_all_from_static

This symbol would normally not be exposed, but it is used from the
Python plugin provider, which is conceptually part of libbabeltrace2,
but implemented as a separate shared library.  Add a comment to explain
that.

Change-Id: Ie6e14edc30489ab6f8c6aadba32a3ceb9726e3b0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2406
Tested-by: jenkins <jenkins@lttng.org>

lib: make bt_object_pool symbols hidden

bt_object_pool_initialize and bt_object_pool_finalize are not supposed
to be exposed by the shared library, so make them hidden.

Change-Id: I072b692f627f4b66b4e5907eeb228d6327d3d70e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2405
Tested-by: jenkins <jenkins@lttng.org>

lib: make symbols in prio-head hidden

These are not supposed to be exposed by the shared library, so make them
hidden.

Change-Id: If1f728f1b536d1be1e38dfff8ca2aba8d4bd1868
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2404
Tested-by: jenkins <jenkins@lttng.org>

tests: add CLI query tests

Add a few simple tests for the query CLI command, using a custom Python
component class.

Change-Id: Ic2b2d3f9f5e0f460f596bd1e4403c250ec2daad5
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2245
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

param-parse: use g_string_append_c instead of g_string_append_printf

... to append a single character.

I did some tests to evaluate the performance impact of this patch, in
terms of number of instructions.  I ran the following command line 20
times and computed the average and standard deviation of the number of
execution instructions.

    $ libtool --mode=execute perf stat -e instructions:u ./src/cli/babeltrace2 query -p 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa=2,' src.ctf.fs yo

                     average     stdev
Without this patch:  1079692     51
With this patch:     1039197     51

This represents a reduction of 3.75% in the number of executed
instructions.

Change-Id: I8109d42d0fa357e47642132a2f3b9581d72bcefa
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2403
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Tested-by: jenkins <jenkins@lttng.org>

lib: remove bt_packet_context_field API

Change-Id: I357daa5915140882dab47acb70570178fb767db8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2390
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

ctf: make msg-iter not use bt_packet_context_field

The bt_packet_context_field API exists so that the CTF message iterator
could store packet context fields in contexts where it doesn't create
trace-ir objects.  For example, in add_ds_file_to_ds_file_group, we use
a message iterator just to sniff the properties of the first packet of a
data stream file.

However, the CTF message iterator doesn't really use it anymore, it just
stores the relevant properties in fields of the ctf_msg_iter structure
and transfers them to the caller in ctf_msg_iter_get_packet_properties.

So the bt_packet_context_field API no longer has a reason to be.  This
patch makes msg-iter stop using it, so it can be removed.

Currently, the packet_context_field is created when needed, which is
just before reading the packet context, in
read_packet_context_begin_state.  A bt_packet is later created in
create_msg_packet_beginning, and the packet_context_field is moved into
the bt_packet.

If we want to get rid of the packet_context_field, it means we need to
create the bt_packet earlier (if not in dry_run mode), so it is
available in read_packet_context_begin_state, so that the packet context
fields are read directly into the bt_packet.  And to create the
bt_packet, the bt_stream needs to be available.  The bt_stream is
currently created just before sending the stream beginning message, in
ctf_msg_iter_get_next_message.

So this patch moves the creation of the bt_stream and bt_packet as early
as possible, just after we have read the packet header, in
after_packet_header_state.  In read_packet_context_begin_state, we can
then find the packet already created in msg_iter->packet.

This change made it necessary to set the dry run flag on the CTF message
iterator in add_ds_file_to_ds_file_group (which should have probably
already been there anyway).  This iterator is used to read the packet
context, which, previously, didn't trigger the creation of the bt_stream
and bt_packet.  But now that we create those earlier, it is necessary
to set the flag to avoid them being created.

It was also needed to change the check in after_packet_context_state to
know whether we are at the first packet, and therefore need to emit a
stream beginning message, or if we are at a subsequent packet, and
therefore need to skip that step.  We would previously check whether the
msg_it->stream field was set, but this is now always true given that we
set the stream earlier.  The only way that I found to fix that is to add
the boolean flag emit_stream_beginning_message in the message iterator.

Change-Id: Ib907480727d00d51aabd89986375d533ad385c96
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2393
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

ctf: remove ctf_msg_iter::set_stream

This field is set but not used.

Change-Id: Ibca38c8421b8d734462077a19da1726a817f3cb6
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2401
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

src.ctf.fs: fix typo in comment

Change-Id: I2a3ee8d019b7dfb966aba3a1e9a5d7fc375eb9e7
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2413
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

lib: mark bt_common_assert_failed as hidden

I noticed that the bt_common_assert_failed symbol was exported.  This is
not desirable, as it's not part of the API, so mark it with BT_HIDDEN.

As a consequence, a bunch of small tests that use BT_ASSERT miss this
symbol, as they were getting it from libbabeltrace2.  Make them depend
on libbabeltrace2-common (and libbabeltrace2-logging) directly to fix
that.

Change-Id: I25ab69e7df08f508389f5c9c3db5571ee7b06d7b
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2402
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

Update version to v2.0.0-rc4

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Idfe7be3fc2eb22bf1e526d6fe6aeb0ce0f5aae88
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2397
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

babeltrace2-source.ctf.fs(7): document the overlapping snapshot feature

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I482a366679f65a577cb7081ce46736ab484858c3
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2395

Revert "Notes de relâche du troisième candidat de relâche"

This reverts commit 98403f26955dc80727dc34974ba1e98d0ead61d1.

Notes de relâche du troisième candidat de relâche

Change-Id: I8d3811ed4fc3196434ee2aebc1af6c542df53824

ctf: remove ctf_msg_iter_set_emit_stream_{beginning,end}_message functions

These functions are now unnecessary.  Before the previous patch
"src.ctf.fs: add and use medops to iterate on a ds_file_group using the
index", the src.ctf.fs component used ctf_msg_iter in such a way that
the iterator would process complete data stream files one after the
other, and was reset between each file.  When starting a data stream
file, the iterator needed to know whether it was the first data stream
file for a given stream, and therefore if it should emit the stream
beginning message.  Conversely, it needed to know if the data stream
file was the last one for the stream, and therefore if it needed to emit
the stream end message.

With the new ctf_fs_ds_group_medops, the ctf_msg_iter reads entire
streams, even if they are spread over multiple data stream files.  We
therefore don't need to say whether a particular iterator run is the
beginning or end of a stream, it always is.

With lttng-live, the iterator also always does a single run so always
needs to emit the stream messages.

This patch also removes the related fields in ctf_msg_iter, as well as
the related states in the state machine.

The only non-obvious thing is: what to do with the call to
set_current_stream in check_emit_msg_stream_beginning_state.  This is
the moment where the message iterator asks the medium to provide the
bt_stream instance for the given stream class and stream instance id.
For now, I moved it to just before we need it the first time, which is
when sending the stream beginning message.  This is probably temporary
anyway, as it is planned that we will be able to just pass the bt_stream
when creating the ctf_msg_iter.

Change-Id: I275d4631ff11612abb46c73312bf133753ae4971
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: use ctf_fs_ds_index_destroy to free index

The function ctf_fs_ds_file_group_destroy frees the ctf_fs_ds_index
structure and its content by hand.  Replace that with a call to
ctf_fs_ds_index_destroy, which is meant for that.

Change-Id: I186b1da9ec63cbdfa7ef7966d1e0e9802827e4f7
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: add and use medops to iterate on a ds_file_group using the index

This patch solves the problem of reading multiple snapshots of the same
tracing session taken quickly.  Taking the snapshots quickly enough can
cause them to overlap, in which case some complete / identical packets
will be found in different snapshots.

As an example, imagine we have three snapshots, and packets labeled from
A to G belonging the same stream instance in all snapshots.  We could
have the following sequence of packets in each snapshot:

  - snapshot 1: A B C D
  - snapshot 2:     C D E F
  - snapshot 3:       D E F G

Babeltrace 1 reads these three snapshots successfully.  In fact, it just
considers them as three different traces, so it will order events
individually.  As a result, events from packet D will be present three
times in the output.  So while it works (as in Babeltrace exits with
status 0), it's probably not what a user would want.

Babeltrace 2 (before this patch) hits the following assert, which
validates that messages produced by iterators never go back in time:

    11-11 15:13:23.874  8329  8329 F LIB/MSG-ITER call_iterator_next_method@iterator.c:872 Babeltrace 2 library postcondition not satisfied; error is:
    11-11 15:13:23.874  8329  8329 F LIB/MSG-ITER call_iterator_next_method@iterator.c:872 Clock snapshots are not monotonic
    11-11 15:13:23.874  8329  8329 F LIB/MSG-ITER call_iterator_next_method@iterator.c:872 Aborting...

This is because Babeltrace 2 groups all CTF traces sharing the same UUID
(which is the case for our three snapshots) and gives them to the same
src.ctf.fs component to read.  The component groups data stream files
from the various snapshots by stream instance id, and sorts them
according to the timestamp of their first event.  It then reads them
sequentially, from end to end, assuming that the start of the second
data stream file is after the end of the first data stream file.  Using
our example above, the src.ctf.fs component would therefore try to read
packets in this order:

    A B C D C D E F D E F G
           ^
   `- ouch!

In this case, we want to read all packets exactly once, in the right
order.

The solution brought by this patch is to iterate on the packets by
following the index, instead of reading all data files from end to end.
Index entries were already de-duplicated by commit

    ctf: de-duplicate index entries

So the index already refers to a single instance of each packet.  We can
therefore use it as a playlist of the packets to read.

The change mainly revolves around adding a new kind of CTF message
iterator medium operations, called ctf_fs_ds_group_medops.  Instead of
the medium being a single data stream file, like in
ctf_fs_ds_file_medops, this new medium is conceptually the sequence of
all packets described by the index of a ctf_fs_ds_group, possibly spread
out in different data stream files.

A new optional medium operation called `switch_packet` is added.  When
the CTF message iterator is done reading a packet, it calls this method,
indicating to the medium that it is at the frontier of two packets.  If
the medium is aware of the packets (like ctf_fs_ds_group_medops is) and
knows that the following packet is not contiguous with the previous
packet, it can reposition its "read head" at the right place (open the
new file, go to the right offset).  Immediatly after calling
`switch_packet`, the message iterator calls the `request_bytes` method,
which allows the medium to return a buffer containing the bytes of the
next packet.

When the packet-aware medium has its `switch_packet` method called but
there are no more packets to read, it returns
CTF_MSG_ITER_MEDIUM_STATUS_EOF.  That brings the message iterator in the
STATE_CHECK_EMIT_MSG_STREAM_END state, which will close the stream.

If the `switch_packet` method is not provided by the medium, the message
iterator just continues, assuming that the bytes of the next packet are
contiguous with those of the previous packet.

The ctf_fs_ds_file_medops medium operations are still necessary for
reading individual ctf_fs_ds_files, when initializing src.ctf.fs
components.  This is needed when building the index from a stream or for
applying tracer fixups.

This simplifies a little bit the interaction between the src.ctf.fs
iterator and the ctf_msg_iter.  Previously, we would read each data
stream file until the message iterator returned EOF.  If it wasn't the
last data stream file, we would reset the iterator to read the next data
stream file.  Functions
ctf_msg_iter_set_emit_stream_{beginning,end}_message were necessary to
indicate to the message iterator whether to send the stream beginning or
end messages, because it had otherwise no idea of whether the data
stream file it is reading is the first one or last one.  The function
ctf_msg_iter_set_medops_data was necessary to swap the data of the
ctf_fs_ds_file_medops to point to the new data stream file.

With the ctf_fs_ds_group_medops, the CTF message iterator only returns
EOF when the stream is done.  The data passed to the
ctf_fs_ds_group_medops has everything it needs to go through all the
packets, so it doesn't need to change.

Change-Id: I72f6d1e09b87414fb83f68cb57abb1f2dc61b439
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

tests: make test_trimmer use bt_cli

... instead of launching babeltrace2.exe by hand.  This makes the
testsuite print the executed command lines, and is therefore easier to
debug.

Change-Id: I82f3984e749b8e6aff38b529ed462373eabd24dc
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: factor out "ds_file_mmap" from "ds_file_mmap_next"

Currently, the logic to mmap a region of a ds_file is contained in
ds_file_mmap_next.  ds_file_mmap_next is also responsible for mapping
the region immediately following the currently mapped region, if there
is a currently mapped region.

I find this double responsibility a bit confusing, especially where
ds_file_mmap_next is used in medop_seek.

This patch extracts a portion of ds_file_mmap_next to a new function
ds_file_mmap, whose job is to ensure a file offset is mapped, and to do
the needful if it isn't.

Change-Id: Ia19725d28cf9c24084a52d16c7a88f30b32ff694
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: remove ctf_fs_ds_file::end_reached field

It is set, but not used for anything, so remove it.

Change-Id: I502fa683e6ab94a54959918978c4ffd683c5768e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: assert that request_sz in medium ops request_bytes is greater than 0

There's a check in medop_request_bytes to return OK if the passed
request_sz is 0.  I don't think that can ever happen, since that
request_sz comes from the max_request_sz parameter when the iterator is
created.  It wouldn't make sense to pass 0.

Change-Id: Ia2c8fb6e56195bcab607c39e366c6b58b3411f8b
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: make ctf_msg_iter_seek assert that the seek callback is not NULL

There should be no reason to call ctf_msg_iter_seek if there is no seek
callback.  It's the user of the ctf_msg_iter who passes medium
operations (at creation time), so it should be aware if there is a seek
callback or not.

This allows removing the UNSUPPORTED values in enum ctf_msg_iter_status
and enum ctf_msg_iter_medium_status.

Change-Id: Ic3591b1e909c7cf0e12d18f40b921a180cf344c8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: assert that msg iter and medium seek offset is valid

I don't think there is ever a valid situation for `offset` to have an
invalid value in ctf_msg_iter_seek or medop_seek.  If the user of the
iterator (e.g.  fs.c) thinks it's cool enough to seek, it should know
about the underlying medium it used to create the msg iter and make sure
not to seek at an invalid position.

This allows removing the INVAL values in enum ctf_msg_iter_medium_status
and enum ctf_msg_iter_status.

Change-Id: I19d48fe751aebf21e60c5cbc16127919a5fa72f3
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: remove ctf_msg_iter_seek_whence parameter to medium ops seek

It is useless, as there is a single value.  If we ever really need to
specify if the given offset is absolute or relative, we can add back a
parameter, as this is an internal API.  In the mean time, it's just an
annoyance.

Change-Id: I3eba79f51d9d5aee82d3bed6ce736459f8dbffa3
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: little status code cleanup

Make ds_file_munmap return an enum ctf_msg_iter_medium_status, so it
integrates well with the rest.  Adjust error handling around the call
sites to be more consistent.

Change-Id: Iea299c782dc215fc64cc3068e58e334f2acb85de
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: don't call ds_file_munmap on failure in ds_file_mmap_next

It seems useless to call ds_file_munmap here.  We can reach the error
label either because:

- the previous call to ds_file_munmap has failed, in which case there's
  no point in retrying
- the mmap call returned MAP_FAILED, in which case there's no point un
  unmapping

Change-Id: Ia11716a967047d32773ab67f51bcf7516489f183
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: make ds_file_munmap assert that ds_file is not NULL

There is no reason for a NULL ds_file to ever be passed to this
function.  Make it assert that the value is not NULL, to catch potential
mistakes.

Change-Id: I9261450f35b0a5c74ba182985ecabe2b31eee0a0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: rename some ctf_fs_ds_file fields

Rename these offset variables to make it a bit more obvious and explicit
what they are relative to.  I find that it helps when reading to code, to
make sure that does the right thing.

Change-Id: Id676a482935f9b17553672160044456cc308883c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src.ctf.fs: remove ctf_fs_ds_file_next

The function ctf_fs_ds_file_next is not really relevant, as it
doesn't deal with anything ds_file-specific anymore.  It only calls the
ctf_msg_iter_get_next_message and translates the result to a
bt_component_class_message_iterator_next_method_status.

I think this job can (and should) be done by fs.c directly, as it's the
primary user/owner of the ctf_msg_iter.

This patch removes ctf_fs_ds_file_next and updates
ctf_fs_iterator_next_one to call ctf_msg_iter_get_next_message directly.

Change-Id: I96a9e1aa9d3c689bdf19f342464e1632c35058ca
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: const-ify a few bt_message parameters

I didn't understand at first why we needed a temporary local variable in
ctf_fs_iterator_next_one, why we couldn't just pass out_msg directly to
ctf_fs_ds_file_next.  If we constify the parameter of
ctf_fs_ds_file_next, we can get rid of the variable.  Doing so has a few
ramifications, it requires to constify the parameter in a few other
functions.  But in the end I think it's all for the greater good.

Change-Id: Ie68249d38dae7a26ab8ce9341b436257644b3b10
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: remove ctf_fs_ds_file::msg_iter field

I find it strange that ctf_fs_ds_file objects knows about the
ctf_msg_iter that iterates on it.  The ctf_fs_ds_file object just
represents some data that is accessible (a data stream file and the
portion currently mmap-ed), but I don't think it should know anything
about the iterating operation.

This patch removes the msg_iter field of ctf_fs_ds_file and adjusts the
code accordingly.

I think this change makes the code a bit clearer when we create
ctf_msg_iter objects.  Currently, we create the msg_iter first with a
NULL medium ops data, then create the ctf_fs_ds_file.  In
ctf_fs_ds_file_create, it sets the ctf_msg_iter's medium ops data to the
ctf_fs_ds_file.

With this patch, we create the ctf_fs_ds_file first.  Then, when we
create the ctf_msg_iter, we can pass the ctf_fs_ds_file as the medium
ops data directly when we create the ctf_msg_iter.

Change-Id: I247f039d225888c059f08ccdccc86abf0816a6c4
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

ctf: de-duplicate index entries

When trace snapshots were taken quickly enough, it's possible for them
to overlap.  Some identical packets will be present in multiple
snapshots.

For example, the first snapshot could contain packets A, B and C while
the second snapshot could contain packets B, C, and D.

When reading those snapshots together with babeltrace, we will want to
present a coherent view of the logical trace that is spread across
multiple snapshots.  That is, we will want to avoid reading the packet
twice.

Currently, we're not considering that case when building the building
the in-memory index.  Processing the two streams above will lead to
those entries in the merged index:

 - Packet A (snapshot 1)
 - Packet B (snapshot 1)
 - Packet B (snapshot 2)
 - Packet C (snapshot 1)
 - Packet C (snapshot 2)
 - Packet D (snapshot 2)

This patch makes it so we only keep a single reference to each packet.
If a packet is duplicated, it doesn't matter to which snapshot the
reference points, since all copies of the packet are identical.  So a
possible outcome of the situation above, with this patch applied, could
be:

 - Packet A (snapshot 1)
 - Packet B (snapshot 1)
 - Packet C (snapshot 1)
 - Packet D (snapshot 2)

So far the index is not used for much, so I don't think that this patch
will have any visible behavior change.  However, an upcoming patch will
build on this to make it so we only read each packet once.

Change-Id: I00962d593b5078253043029902f853e5c3fa0dc5
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>