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>

ctf: read packet sequence number from index

Starting with version 1.1, the CTF index format includes a packet
sequence number.  Read it, if the index minor version is >=1.  The index
major version is already checked to always be 1 above in the code.

If the version is lower than that, store UINT64_MAX in the
packet_seq_num field of the ctf_fs_ds_index_entry structure.

This information will be used in a subsequent patch.

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

Save and restore error in ctf_fs_iterator_next, muxer_msg_iter_do_next

ctf_fs_iterator_next and muxer_msg_iter_do_next are written in such a
way that if they successfully accumulate some messages, then hit an
error, they will return OK to make sure that those messages propagate
downstream in the graph, before the error stops the execution.  This
assumes that the subsequent call to _next will fail again.

This poses a problem: when it returns OK in this case, it also leaves
the current thread error set.  This is a postcondition breach.

One solution would be to simply clear the error in that case.  This
would still rely on some error (perhaps the same, perhaps not) to be hit
again  on the next _next call.   What I don't like with that solution is
that the behaviour of the iterator, if we call _next again after a
failure, is hard to predict.  We could hit the same error if we are
lucky, we could hit another error, we could hit an assertion because we
are in an unexpected state, etc.

Instead, this patch implements the following solution: return OK, but
take the error from the current thread and save it in the iterator.  The
next time _next is called, that error is moved back to the current
thread and the error status is returned.  This ensures that the error
the user will see is the one that originally caused the problem.

A test is added with a trace containing two valid events followed by an
invalid one.  We expect to see the two events printed by the details
sink, as well as the error about the invalid event class id.

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

muxer: append causes on some _next iterator method code paths

The goal of this patch (on top of the fact that having more error causes
is nice) is to ensure that if an error status is returned to
muxer_msg_iter_do_next, an error is set on the current thread.  This
will help for the next patch, which will save and restore the error in
this function.

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

lib: save and restore current thread error when calling destruction listeners and finalize methods

Consider the following chain of events:

- User creates and runs a graph
- An error happens when running the graph, _ERROR is returned and the
  current thread has an error
- The user does put_ref on the graph to clean up, deliberately leaving
  the error on the current thread so it's processed above on the stack.
- When calling put_ref, a trace destruction listener is called.
- After calling the trace destruction listener, the lib asserts that no
  error is set (BT_ASSERT_POST_NO_ERROR), which is not true, so the
  assert fails.

In this case, the user of the graph would have to take the error, do the
put_ref and restore the error, if it wants to leave it for a function
higher in the stack.  However, it is such a common scenario to hit an
error, do some put_ref to clean up and return the error, that it would
be very heavy (and error-prone) to have to do this all the time.

This patch makes it safe to call put_ref with an error set by wrapping
all the user code that can be called as a consequence of a put_ref
(destruction listeners and finalize methods) with an error take/move.

Before calling the user destruction listener or finalize method, the lib
saves the error on the stack.  The user callback is therefore called
with no error set and the lib can still validate with
BT_ASSERT_POST_NO_ERROR that it leaves no error on the current thread
after returning.  The error is then moved back.

We could say that put_ref now become "error-neutral", in that they can
be called with an error set on the current thread, and they won't modify
it.

Change-Id: I8c7a5429d53073483b9e03f0ec654c826466ee4e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2385
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

Cleanup: src.ctf.lttng-live: move function declarations around

* Remove declarations of `lttng_live_remove_stream_iterator()` and
`lttng_live_add_stream_iterator()` functions as they are not used.

* Move `lttng_live_need_new_streams()` function to viewer-connection.c
as it's only used in this file.

* Move declaration of `lttng_live_create_viewer_session()` to
viewer-connection.h

* Namespace `lttng_live_session` related functions with the
`lttng_live_session_` prefix.

* Rename `lttng_live_borrow_trace()` function to
`lttng_live_session_borrow_or_create_trace_by_id()`

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

src.ctf.lttng-live: replace usage of `calloc()` by `g_new0()`

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

Fix: src.ctf.lttng-live: `fwrite()` does not set `errno`

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

src.ctf.lttng-live: don't try to detach after socket error

Background
==========
The message iterator `_finalize()` method typically needs to detach from
the live session (LTTNG_VIEWER_DETACH_SESSION) so that the relay daemon
can free up the associated resources.

Issue
=====
In the case we encounter a socket error during the `_next()` method,
we don't want a component to try to send a detach command to the relay
during the `_iter_finalize()` method as this will surely lead to another
error. The annoying thing is that this other error would pollute the
error cause stack that lead to the first error.

Solution
========
Close the socket when we encounter an error and check in the
`lttng_live_detach_session` function if the socket it valid before
sending a _DETACH_SESSION command.

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

Fix: src.ctf.lttng-live: decode metadata even on _STATUS_CLOSED

When receiving a _GET_ONE_METADATA_STATUS_CLOSED from the
`lttng_live_get_one_metadata_packet()` function it means that the
metadata stream was closed by the relay and we will no longer get any
new metadata. But, it also means that everything we received so far is
valid and can be decoded.

So rather than returning _LIVE_ITERATOR_STATUS_END the right thing to do
is to go on with the decoding of the metadata packets.

Also did the following cleanups:
  * Merge the `lttng_live_metadata::trace` and
  `lttng_live_trace::new_metadata_needed` into a new tri-state enum
  `lttng_live_metadata_stream_state` in the `lttng_live_trace` struct.
  Those two fields were tightly linked and it's simpler to have a single
  field.

  * Remove the "poking of the trace metadata" using the
  `new_metadata_needed` as it's no longer necessary since all references
  are now borrowed.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I3e18365d5cfeaa77935409bdfe8fdda52fa5b636
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2351

Fix: src.ctf.lttng-live: removing trace when not all streams are done

The following commit wrongfully added the removal of a live trace
object:
  commit c28512ab93b16501a8b0da494f0a03e5f0f22662
  Author: Francis Deslauriers <francis.deslauriers@efficios.com>
  Date:   Mon Oct 28 14:49:08 2019 -0400

      src.ctf.lttng-live: make `lttng_live_get_one_metadata_packet()` return status

The live trace objects should only be removed once all its stream
iterator have ended. We do that in next_stream_iterator_for_session()
function.

So, roll back this change but keep the switch statement so it's clearer
what are the possible outcomes of the preceding function call.

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

src.ctf.lttng-live: Add logging statements across the component class

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ib24204d48e43b91a5f0ad0111fe4e9d0abef5613
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2349

Cleanup: src.ctf.lttng-live: rename live_iterator_status printing function

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

Cleanup: src.ctf.lttng-live: add `lttng_live_msg_iter_create()`

One small step to increase readability of this component class.

Change-Id: I4e690d96b85575847314e30ea9223806f4a049dc
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2343

src.ctf.lttng-live: make component class handle interruptions

Currently, if a `src.ctf.lttng-live` component is interrupted by a
signal during a I/O syscall, it will return an _ERROR status downstream.
This is wrong for two reasons:
  1. Some signals are not problematic (e.g. SIGUSR1) so we want to
     continue as normal, and
  2. we want to return an _AGAIN status when we receive a SIGINT
     signal.

With this commit, when getting interrupted during a `recv()` or `send()`
call, we check if the graph is now in the interrupted state and return
the _AGAIN status if this case.
This commit changes the semantic of the lttng_live_{recv, send}
functions to make them return a status (_OK, _INTERRUPTED, or _ERROR).
The _OK status is only returned if the entire message was received or
sent. This allows us to remove many checks of the length of received or
sent data.

With this commit, once a live iterator is interrupted by SIGINT and
returns _AGAIN it's not in a state that can be recovered if the graph was
to be run again. Further work is needed to make this component class
restartable.

So to prevent user of the graph to call the _NEXT method after an
interruption, we mark the live iterator as interrupted (with the
`was_interrupted` boolean field) and return an error if it's the case.

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

Fix: src.ctf.lttng-live: checking `errno` value on all errors

Issue
=====
We check the errno value even in cases where the errno is not set by the
function that witnessed the error.
For example, we have a `goto error;` after the
`lttng_live_get_one_metadata_packet() call.

Solution
========
Move the errno check right after the functions that set it on error.

Note
====
This erroneous change was introduced by this commit:
  commit c28512ab93b16501a8b0da494f0a03e5f0f22662
  Author: Francis Deslauriers <francis.deslauriers@efficios.com>
  Date:   Mon Oct 28 14:49:08 2019 -0400

      src.ctf.lttng-live: make `lttng_live_get_one_metadata_packet()` return status

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

Cleanup: comp-logging.h: template `BT_COMP_OR_COMP_CLASS_LOG*` macros

This commit is motivated by the need to have the
`BT_COMP_OR_COMP_CLASS_LOGW_ERRNO()` logging macro in a future commit.

It adds the following template macros to simplify add logging macros for
other logging levels:
  BT_COMP_OR_COMP_CLASS_LOG
  BT_COMP_OR_COMP_CLASS_LOG_ERRNO

I didn't add the `BT_COMP_OR_COMP_CLASS_LOG_APPEND_CAUSE` template as
it's less likely that we will need it for logging levels other than
`BT_LOG_ERROR`.

Also, to be uniform across the file, I moved all the `_lvl` parameter to
the first position for all macros in this file.

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

lib: make bt_value_map_foreach_entry_{const_}func() return a status code

This patch makes the bt_value_map_foreach_entry_func() and
bt_value_map_foreach_entry_const_func() types return status codes
instead of `bt_bool`.

The available status codes are `OK` (continue the loop, like returning
`BT_TRUE` before this patch), `ERROR`, `MEMORY_ERROR`, and `INTERRUPT`
(break the loop, like returning `BT_FALSE` before this patch).

When the user function returns
`BT_VALUE_MAP_FOREACH_ENTRY_FUNC_STATUS_ERROR`,
bt_value_map_foreach_entry() returns
`BT_VALUE_MAP_FOREACH_ENTRY_STATUS_USER_ERROR` (set to the new global
status code `__BT_FUNC_STATUS_USER_ERROR`). This makes it possible to
distinguish between an internal library error and a user function error.

The purpose of this patch is to make it possible for a user function to
append an error cause to the current thread's error the same way other
user functions do: append the cause and return an error status.

For example, consider this scenario:

1. User calls bt_value_map_foreach_entry() with a user function and user
   data which contains a status member.

2. User function calls a library function which fails. The library
   function appends a cause to the current thread's error.

3. User function appends a cause to the current thread's error.

4. User function sets the user data's status member to the failing
   library function's status and returns `BT_FALSE` to interrupt the
   loop.

5. bt_value_map_foreach_entry() returns
   `BT_VALUE_MAP_FOREACH_ENTRY_STATUS_INTERRUPTED` (not an error
   status).

6. The caller of bt_value_map_foreach_entry() interprets
   `BT_VALUE_MAP_FOREACH_ENTRY_STATUS_INTERRUPTED` as an error because
   the user data's status member has an error value.

With this patch, it becomes:

1. User calls bt_value_map_foreach_entry() with a user function.

2. User function calls a library function which fails. The library
   function appends a cause to the current thread's error.

3. User function appends a cause to the current thread's error.

4. User function returns `BT_VALUE_MAP_FOREACH_ENTRY_FUNC_STATUS_ERROR`
   or `BT_VALUE_MAP_FOREACH_ENTRY_FUNC_STATUS_MEMORY_ERROR`, depending
   on the failing library function's status, which breaks the loop.

5. bt_value_map_foreach_entry() appends a cause to the current thread's
   error and returns `BT_VALUE_MAP_FOREACH_ENTRY_STATUS_USER_ERROR` or
   `BT_VALUE_MAP_FOREACH_ENTRY_STATUS_MEMORY_ERROR`.

The latter seems more natural to me.

In Python, nothing wraps bt_value_map_foreach_entry() directly, so I
just converted bt_value_map_get_keys_cb() to return the appropriate
status instead of `BT_TRUE` or `BT_FALSE`. In other words,
bt2.utils._handle_func_status() does not need any change because it will
never receive `native_bt.__BT_FUNC_STATUS_USER_ERROR`.

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

Emit dedicated bright terminal color codes if supported

Some terminals support having a bold normal foreground color which is
_not_ bright. kitty is one of them, as well as GNOME Terminal with the
fairly recent "Show bold text in bright colors" option turned off (or
when calling the underlying vte_terminal_set_bold_is_bright() function
with the appropriate value).

An easy test is to execute:

    $ echo -e "\033[31mTHIS\n\033[1mTHAT\033[0m"

and compare the colors of both lines: if they are the same, then the
terminal supports non-bright bold foreground colors.

For those terminals, the way to have a non-bold bright color is to emit
the dedicated SGR codes 90 to 97:

    $ echo -e "\033[91mHELLO\033[0m"

Some terminals can print non-bold bright colors, but cannot print
non-bright bold colors.

This patch makes the Babeltrace project emit different color codes
depending on the connected terminal and the new
`BABELTRACE_TERM_COLOR_BRIGHT_MEANS_BOLD` environment variable's value:

kitty or `BABELTRACE_TERM_COLOR_BRIGHT_MEANS_BOLD` is `0`:
    Output bright colors using dedicated SGR codes 90 to 97.

Otherwise:
    Output bright colors with bold + SGR codes 30 to 37.

This patch effectively makes the Babeltrace modules emit correct bright
color codes for kitty.

To change as little as possible, I updated all the sites which emit a
bold code followed by a foreground color code to emit instead a bold
color code followed by a bright foreground color code. The only drawback
with the current approach is that, for non-kitty terminals, two bold
color codes are emitted for those cases (the real bold and the
brightening bold).

This means all bold colored text is also bright currently. Eventually we
can start decoupling the bold attribute from bright colors if need be,
although this means users must have a supporting terminal.

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

lib: add pre condition asserts to check current thread has no error

The user must not call an API function while the current thread has an
error set, especially a function that can itself fail and set an error.
Otherwise, causes from two unrelated errors will end up in the same
stack, which will produce some misleading information.

Add a precondition assertion to all API functions that are likely to
themselves set an error to the current thread, to verify that there is
not error on the current thread when they are called.

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

lib: add post condition assertions for current thread error after user functions

It is a logic error and a post condition breach for a user function to
return a non-error status code (>= 0) but leave an error set on the
current thread.  Add some assertions after each point where we call a
user function to catch this kind of mistake.

The macro BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS is meant to be
used in the very fast path (consume/next callbacks).  The macro
BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS is used elsewhere.

The macros are written in such a way that if they cause an abort, the
error will still be present in thread_error, so accessible to a
debugger.

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

lib: clear error in clock_snapshots_are_monotonic_one

When bt_clock_snapshot_get_ns_from_origin is called in
clock_snapshots_are_monotonic_one, it can return OVERFLOW_ERROR, in
which case it also sets the current thread error.
clock_snapshots_are_monotonic_one doesn't report an error status to its
caller (it's not its goal), so it should not leave the error there.

This is exercised when running Python test
ClockSnapshotTestCase.test_clock_class.

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

tests: clear error in test_simple_sink

When test_simple_expect_run_once_status is used to test an error case,
it leaves the current thread error set.  The next test then starts with
an error already set, which is not right.

Clear the error at the end of test_simple_expect_run_once_status.  And
while at it, validate that we have an error set exactly when an error
status is returned.

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

lib: append error in simple_sink_consume only if error status

If the consume_func returns _AGAIN, we will append an error cause.  That
is wrong because _AGAIN is not an error cause.  Check for negative
status values instead of just != OK.

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

src.ctf.fs: rename pc_msg_iter parameter to self_msg_iter

This is a leftover of:

    f30762e5afa0 ("Rename pc_msg_iter fields to self_msg_iter")

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

Cleanup: cli: move LOGE statements closer to the source

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

cli: exit with status 2 when interrupted by SIGINT

With this commit, users of the CLI issuing a ctrl+c to interrupt a
running graph will not get an error cause stack printed to stderr.
Instead, they will now simply get a non-zero exit status to signify that
the execution of the command did not complete as expected.

We preferred this approach, because we consider it unexpected that a
user who willingly pressed ctrl+c to stop the command ends up getting an
error cause stack printed to the CLI.
As the execution did not complete normally, we also did not want the
command to exit with status 0 in such cases. Exiting with status code 1
was also not appropriate as it is typically used to signify an error.

In sum, the middle ground we found, is to exit with status code 2 and
not print an error message.

This commit adds tests to confirm that the right exit status is produced
by the CLI when a graph is interrupted, is erroring, or is returning
successfully. To do this, this commit adds a custom Python source
component class that takes as parameter the name of the scenario to
produce during its iterator's first `_next()` call. The iterator then
does the needful to put the graph in the right condition. We then test
if the CLI exits with the expected status.

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

Ignore -Wcast-function-type warning

We get this when building on CentOS 8:

bt2/native_bt.c:1819:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"disown",  (PyCFunction)SwigPyObject_disown,  METH_NOARGS,  (char *)"releases ownership of the pointer"},
                       ^
bt2/native_bt.c:1820:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"acquire", (PyCFunction)SwigPyObject_acquire, METH_NOARGS,  (char *)"acquires ownership of the pointer"},
                       ^
bt2/native_bt.c:1823:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"next",    (PyCFunction)SwigPyObject_next,    METH_NOARGS,  (char *)"returns the next 'this' object"},
                       ^
bt2/native_bt.c:1824:23: error: cast between incompatible function types from ‘PyObject * (*)(SwigPyObject *)’ {aka ‘struct _object * (*)(struct <anonymous> *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"__repr__",(PyCFunction)SwigPyObject_repr,    METH_NOARGS,  (char *)"returns object representation"},
                       ^

See comment in configure.ac for detailed explanation.  It would have
been nice to just disable it when building native_bt.c, but I am not
sure how to do that in a way that's compatible with all compilers and
that is not overly complicated.  So let's just disable it globally for
now.

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

Cleanup: src.ctf.lttng-live: remove redundant _APPEND_CAUSE()

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

cli: set `bt_config::command_name` field for all commands

This field is used in babeltrace2.c to print the pretty print the
command name in different logging messages.

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

src.ctf.fs: make log macros of data-stream-file.c more generic

The logging macros in data-stream-file.c currently assume that all the
contexts where a logging statement is used will have a self component at
ds_file->self_comp and a logging level at ds_file->log_level.  In a
future patch, I'd like to use a logging statement where there is no
reason to pass a ds_file variable.

Make the macros more generic by having them refer to just self_comp and
logging_level.

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

ctf: check version of LTTng trace index

If LTTng was to start producing trace index files with major version 2,
it would presumably be because the format has become
backwards-incompatible.  Babeltrace would currently try to parse it like
it parses version 1.x index files, and that would likely not give good
results.

This patch makes Babeltrace check the major version of LTTng trace index
files before parsing them, and ignore them if the major version is not 1.

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

ctf: save self_msg_iter in ctf_msg_iter when creating it

I find it strange that we assign ctf_msg_iter::self_msg_iter in
ctf_msg_iter_get_next_message.  The self_msg_iter associated to the
ctf_msg_iter is the same throughout its lifetime, and known at creation
time, so we can assign it once at creation time.

This patch adds a self_msg_iter to ctf_msg_iter_create to do that.  It
also removes the self_msg_iter parameter from
ctf_msg_iter_get_next_message, since it is not needed anymore.

Some ctf_msg_iter instances are created not in the context of a
bt_self_message_iterator (e.g. when we create a src.ctf.fs component
and we need to build an index).  For those cases, we pass NULL.

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

ctf: rename bt_msg_iter to ctf_msg_iter

The bt_msg_iter structure is specific to iterating on CTF messages, so
it would be more appropriate for it to be called ctf_msg_iter.  Rename
it, as well as related types and functions.

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

ctf: rename bt_msg_iter::msg_iter to self_msg_iter

This follows the naming convention that I have used so far.  It helps
knowing which kind of msg iter we are talking about, given that there
are multiple kinds.

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

Rename pc_msg_iter fields to self_msg_iter

The pc prefix means "private connection", which is not a concept
present in the API anymore.  Rename to "self_msg_iter", which is a more
contemporary term.

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

bt_common_abort(): optionally execute a custom command before aborting

This patch makes bt_common_abort() execute the value of the
`BABELTRACE_EXEC_ON_ABORT` environment variable, if it's set, as a shell
command line if the setuid/setgid flags are NOT set (for security).

The function uses g_spawn_command_line_sync() which claims to parse the
command line string like a UNIX 98 shell would, so that's what I wrote
in the environment variable's description in the manual page.

You can use this to execute any command when any part of the Babeltrace
project would abort, for example report it to some system.

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

Add bt_common_abort() and use it instead of abort() directly

This patch adds bt_common_abort() which, for the moment, only calls
abort().

This patch also replaces all the calls to abort() with calls to
bt_common_abort().

The purpose is to control how all the parts of Babeltrace abort,
eventually adding other actions before calling abort() for example.

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

.gitignore: add missing `/tests/param-validation/test_param_validation`

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

ctf: fix typo

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

Update version to v2.0.0-rc2

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4fcfc6d3e3ef76c16760b599101a8275a9827b28

Cleanup: src.ctf.lttng-live: add missing `#include <stdint.h>`

Use stdint.h whenever only uint*_t are need and inttypes.h when
printf-like identifier are need (e.g. %PRIu64).

And reorder some #include statements.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ic69636e4ac316a86f88ba0bdd8ad435b38db0e6a
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2313

Cleanup: src.ctf.lttng-live: remove usage of `bt_object`

The live_viewer_connection objects are never shared and we can know that
by seeing that there is no to _get_ref() in this entire component class.

This commit also removes the need for the BT_OBJECT_DONT_LOG macro added
by:
    commit 9358ceb38baa2d50a69d7ee580463a2ddba25465
    Author: Philippe Proulx <eeppeliteloop@gmail.com>
    lib/object.h: log conditionally to `BT_OBJECT_DONT_LOG` undefined

so remove that to.

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

lib: lib-logging.c: `Babeltrace library` -> `libbabeltrace2`

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

ctf: msg-iter.c: rename `notit` to `msg_it`

Messages used to be called notifications. The CTF message iterator used
to be called notification iterator. This explains why the msg-iter.c
file has tons of variables and parameters named `notit`.

I believe it's time to rename them to a proper name: `msg_it`.

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

ctf: msg-iter.c: use `_APPEND_CAUSE` variants of logging macros

This commit also changes usage of logging level macros `LOGW` in this
file to use `LOGE` which is more appropriate when returning
_STATUS_ERROR. All callers of this API consider _STATUS_ERROR as
un-recoverable.

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

src.ctf.lttng-live: use `_APPEND_CAUSE` variants of logging macros

This commit adds `BT_COMP_CLASS_LOGE()` and
`BT_COMP_OR_COMP_CLASS_LOGE()` macros for convenience.

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

src.ctf.lttng-live: make `lttng_live_attach_session()` return status

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

src.ctf.lttng-live: make `lttng_live_get_one_metadata_packet()` return status

This is needed to differentiate real critical errors from relay daemon
from errors which are handled.

The relay daemon can return the LTTNG_VIEWER_METADATA_ERR status to
convey that the requested metadata stream is not found of the relay.
This can happen during normal operations if the associated trace is no
longer active. Short lived UST apps in per-pid session may produce such
scenario. In these cases, simply remove the live trace from the session
and go on with the iteration.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I452e63bd12a3c58d518726e3d178be241464bc2a
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2277

logging: ignore -Wundef in log.c

When building with -Wundef enabled, I get this warning:

    /home/smarchi/src/babeltrace/src/logging/log.c: In function ‘put_tag’:
    /home/smarchi/src/babeltrace/src/logging/log.c:455:15: error: "_BT_LOG_MESSAGE_FORMAT_MASK__TAG" is not defined, evaluates to 0 [-Werror=undef]
      _PP_CONCAT_3(_BT_LOG_MESSAGE_FORMAT_MASK_, _, field)
                   ^
    /home/smarchi/src/babeltrace/src/logging/log.c:350:30: note: in definition of macro ‘_PP_PASTE_3’
     #define _PP_PASTE_3(a, b, c) a ## b ## c
                                  ^
    /home/smarchi/src/babeltrace/src/logging/log.c:455:2: note: in expansion of macro ‘_PP_CONCAT_3’
      _PP_CONCAT_3(_BT_LOG_MESSAGE_FORMAT_MASK_, _, field)
      ^~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.c:470:3: note: in expansion of macro ‘_BT_LOG_MESSAGE_FORMAT_MASK’
      (_BT_LOG_MESSAGE_FORMAT_MASK(field) & _BT_LOG_MESSAGE_FORMAT_FIELDS(format))
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.c:1151:6: note: in expansion of macro ‘_BT_LOG_MESSAGE_FORMAT_CONTAINS’
     #if !_BT_LOG_MESSAGE_FORMAT_CONTAINS(TAG, BT_LOG_MESSAGE_TAG_FORMAT)
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That code comes from zf_log and is a big mess of macros, I don't know
how to fix that.  However, I'd like to enable -Wundef, given that it has
found a relatively important bug (see commit 9103e903a8 "Fix: define
macros for logging levels").  So, silence -Wundef just for that
particular spot.

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

lib: Make `bt_version_get_*() return unsigned int

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

Rename `BT_RANGE_SET_` to `BT_INTEGER_RANGE_SET_`

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

bt2: validate parameters to _TraceClass.create_stream_class before creating the native object

When creating a stream class (in _TraceClass.create_stream_class), we
start by creating the native object, then assign each property passed
as arguments to create_stream_class.

If the value of a parameter is invalid (e.g. wrong type), we will raise
an exception.  The problem is that at this point, the stream class
object has already been created and added to its parent, the trace
class.  This leaves the user in an uncomfortable position, where a
stream class has been created, with the parameters only partially
applied, while an exception was raised.

Instead, the stream class native object should be created only once we
know that all the parameters are valid.  This patch makes it so we
validate all parameters before calling the native creation function.

Given that the setters in _StreamClass are internal and only used in
_TraceClass.create_stream_class, we don't need to validate the values of
the parameters again in them.

I have added tests for two conditions that weren't tested, when passing
a wrong parameter type to assigns_automatic_event_class_id and
assigns_automatic_stream_id.

Otherwise, I added assertions to the test to make sure that when the
stream class Python object couldn't be created, no stream class object
was added as a child of the trace class.

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

tests: use assertRaisesRegex instead of assertRaises in test_stream_class.py

Using assertRaises is not very robust.  We can expect a ValueError to be
raised, but in reality the test can pass because a ValueError different
than the one we are expecting is raised, and we don't actually test what
we mean to.

Use assertRaisesRegex throughout test_stream_class.py, which validates
the exception message in addition to the type.

I have also updated a few exception messages in the process, where I
thought they could be made clearer.

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

Cleanup: add `#include <stdbool.h>` whenever `bool` type is used

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I1984ff361c8197efd44928a6fc6889fa60cfdc33
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2303

lib: remove `BT_GRAPH_RUN_STATUS_END`

This patch makes bt_graph_run() return `BT_GRAPH_RUN_STATUS_OK` when the
graph is done running instead of `BT_GRAPH_RUN_STATUS_END`, as one of
them is redundant here and keeping an OK status looks like the right
decision.

bt_graph_run_once() is different: it returns
`BT_GRAPH_RUN_ONCE_STATUS_OK` once it's done running a single time, and
eventually `BT_GRAPH_RUN_ONCE_STATUS_END` when all the sink components
are ended.

In bt2.Graph.run(), the special case for `bt2.Stop` is removed as
`status` is never `BT_GRAPH_RUN_STATUS_END`.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I9d642292083c3bce0b7be263242f5b23d3713735
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2281
CI-Build: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

Cleanup: ctf: msg-iter.c: rename `ret` to `status`

This is to follow the coding style of the rest of the file and most of
the project.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ibdc1adcc8eef7d95df2c69113bc20228c4d00b9b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2247
Tested-by: jenkins <jenkins@lttng.org>

Cleanup: ctf: remove duplicated logging statement

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ica4fa11646a59ddbfb6cd4c2a9cd108846ffd6dc
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2246
Tested-by: jenkins <jenkins@lttng.org>

Cleanup: msg-iter.c: make `create_msg_*()` return msg

Those functions have a void return type but are returning the created
message using an output parameter.

Make them return the message instead.

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

Cleanup: src.ctf.lttng-live: NULL check already done in `_is_canceled()` func

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

cli: Remove unnecessary NULL check in print_value_rec

There's no reason for `value` to legitimately be NULL here.  Replace
that with an assert so we can catch eventual mistakes.

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

source.ctf.lttng-live: clean-up: don't restart session iteration

Don't restart the iteration on a session array when a session has to
be removed. The contents of the array's current position are replaced
by the contents of the last position in the array. Thus, we can
continue the iteration from the current index.

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

source.ctf.lttng-live: clean-up: don't restart stream iteration

Don't restart the iteration on a stream array when a stream has
to be removed. The contents of the array's current position are
replaced by the contents of the last position in the array. Thus,
we can continue the iteration from the current index.

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

Fix: source.ctf.lttng-live: assertion on equal messages

The following assertion fails when consuming a per-pid trace
of short-lived applications.
 (╯°□°)╯︵ ┻━┻  muxing.c:849: common_muxing_compare_messages(): Assertion `left_msg != right_msg` failed.

The live source performs a muxing step during which it can see that a
trace has ended. When that happens, the trace is removed from the
array of traces and the iteration on that array resumes from the
beginning. This causes the message comparator to assert as two
identical messages can be compared.

Not reseting the trace index to 0 causes the iteration to continue
from the same position in the array. This is fine since the "fast"
variant of the glib pointer array removal function replaces the
removed pointer by the last one. This is both more efficient and
solved the problem of comparing a message to itself (the oldest
message) during a second pass on the traces array.

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

Add compile_commands.json to .gitignore

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

Cleanup: src.ctf.lttng-live: coding style

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

Fix: add missing decoder-packetized-file-stream-to-buf.h

The previous commit (b8433fc, "Fix: add missing
decoder-packetized-file-stream-to-buf.h") was missing this file.

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

Fix -Wmissing-prototypes/-Wmissing-declarations warnings

I think this warning is useful because it catches a lot of instances
where we missed including "foo.h" in "foo.c" or where we should make a
function static.

There are changes in this patch I am not sure about:

bt_ctf_event_borrow_stream: there doesn't seem to be any "borrow"
functions in the CTF writer API, only get (which return new references),
so I presume this is one is not meant to be exposed.  I made it static.

_bt_ctf_event_freeze: is not used anywhere, starts with an underscore,
so I presume it was meant to be internal, so I removed it.

bt_ctf_event_common_set_payload: it sounds like something that was meant
to be exposed internally, so I added a declaration to the internal
event.h.

bt_ctf_event_class_get_payload_type_*: they seem like functions that are
meant to be exposed in the external API, so I added declarations in the
external event.h.

bt_ctf_field_type_enumeration_mapping_iterator_next: it probably needs
to be exposed to make the functions which return a
bt_ctf_field_type_enumeration_mapping_iterator useful.

bt_ctf_field_type_enumeration_mapping_iterator_signed_get and
bt_ctf_field_type_enumeration_mapping_iterator_unsigned_get: same as
above.

bt_ctf_trace_visit: I guess it's meant to be exposed, so I added a
declaration in the internal trace.h.

bt_self_component_port_input_message_iterator_borrow_component_const: I
guess this should not exist, because hand out const versions of
bt_self_component_port_input_message_iterator?  I removed it.

bt_plugin_python_create_all_from_file: adding an include of
python-plugin-provider.h in python-plugin-provider.c revealed that the
declaration was way out of sync with the definition, I've tried to fix
that (and succeeded!).

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

ctf: Remove redundant declarations of lexer/parser functions

I get these warnings:

      CC       libctf_parser_la-lexer.lo
    lexer.c:857:17: error: redundant redeclaration of ‘yyunput’ [-Werror=redundant-decls]
      857 |
          |                 ^
    /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/lexer.l:55:13: note: previous declaration of ‘yyunput’ was here
       55 | static void yyunput (int c, register char * yy_bp , yyscan_t yyscanner)
          |             ^~~~~~~

      CC       libctf_parser_la-lexer.lo
    lexer.c:871:12: error: redundant redeclaration of ‘input’ [-Werror=redundant-decls]
      871 | #else
          |            ^
    /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/lexer.l:55:12: note: previous declaration of ‘input’ was here
       55 | static int input (yyscan_t yyscanner) __attribute__((unused));
          |            ^~~~~

      CC       libctf_parser_la-parser.lo
    /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:63:5: error: redundant redeclaration of ‘yyparse’ [-Werror=redundant-decls]
       63 | int yyparse(struct ctf_scanner *scanner, yyscan_t yyscanner);
          |     ^~~~~~~
    In file included from /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/parser-wrap.h:41,
                     from /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:47:
    parser.h:192:5: note: previous declaration of ‘yyparse’ was here
      192 | int yyparse (struct ctf_scanner *scanner, yyscan_t yyscanner);
          |     ^~~~~~~

Because of the __attribute__((unused)), when removing the declarations
of yyunput and input from lexer.l, I expected to get a
"-Wunused-function" warning.  But that doesn't seem to be the case (even
when manually enabling -Wunused-function), so I'm giving a shot at
removing those declarations, since they may not be essential.

The yyparse declaration is not essential, since there's one already in
the generated parser.h.

There is a slight possibility that older bison versions (I'm using
3.4.2) produce different results, but I can't easily test right now.

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

ctf-writer: Fix -Wredundant-decls warning

I get this warning:

      CC       clock.lo
    In file included from /home/simark/src/babeltrace/src/ctf-writer/stream-class.h:42,
                     from /home/simark/src/babeltrace/src/ctf-writer/trace.h:44,
                     from /home/simark/src/babeltrace/src/ctf-writer/clock.h:32,
                     from /home/simark/src/babeltrace/src/ctf-writer/clock.c:41:
    /home/simark/src/babeltrace/src/ctf-writer/utils.h:40:13: error: redundant redeclaration of ‘bt_ctf_get_byte_order_string’ [-Werror=redundant-decls]
       40 | const char *bt_ctf_get_byte_order_string(enum bt_ctf_byte_order byte_order);
          |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    In file included from /home/simark/src/babeltrace/src/ctf-writer/field-types.h:42,
                     from /home/simark/src/babeltrace/src/ctf-writer/stream-class.h:39,
                     from /home/simark/src/babeltrace/src/ctf-writer/trace.h:44,
                     from /home/simark/src/babeltrace/src/ctf-writer/clock.h:32,
                     from /home/simark/src/babeltrace/src/ctf-writer/clock.c:41:
    /home/simark/src/babeltrace/src/ctf-writer/writer.h:67:13: note: previous declaration of ‘bt_ctf_get_byte_order_string’ was here
       67 | const char *bt_ctf_get_byte_order_string(enum bt_ctf_byte_order byte_order);
          |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

The implementation is in writer.c and we have a declaration in writer.h.
The declaration in utils.h seems superfluous.

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

Fix: ctf-writer: field_type_common_has_known_id always returns true

I get this when building with gcc:

      CC       event.lo
    In file included from /home/simark/src/babeltrace/src/ctf-writer/event.h:42,
                     from /home/simark/src/babeltrace/src/ctf-writer/event.c:45:
    /home/simark/src/babeltrace/src/ctf-writer/fields.h: In function ‘field_type_common_has_known_id’:
    /home/simark/src/babeltrace/src/ctf-writer/fields.h:291:53: error: logical ‘or’ of collectively exhaustive tests is always true [-Werror=logical-op]
      291 |  return (int) ft->id > BT_CTF_FIELD_TYPE_ID_UNKNOWN ||
          |

Indeed, the logical expression there always returns true.  Change the ||
for a &&, which is what I think was meant here.

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

Fix -Wshadow warnings

Fix warnings of this type:

      CC       event.lo
    /home/simark/src/babeltrace/src/ctf-writer/event.c: In function ‘bt_ctf_event_common_create_fields’:
    /home/simark/src/babeltrace/src/ctf-writer/event.c:135:21: error: declaration of ‘create_field_func’ shadows a global declaration [-Werror=shadow]
      135 |   create_field_func create_field_func,
          |   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
    In file included from /home/simark/src/babeltrace/src/ctf-writer/event.c:45:
    /home/simark/src/babeltrace/src/ctf-writer/event.h:89:17: note: shadowed declaration is here
       89 | typedef void *(*create_field_func)(void *);
          |                 ^~~~~~~~~~~~~~~~~

Simply rename the types so they don't have the same name as the
parameters, suffix the types with `_type`.

I have added -Wno-shadow when building native_bt.c (in the Python
bindings), see the comment in Makefile.am for more details.

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

debug-info: fix one -Wnull-dereference warning

When building with gcc at -O2, I get:

      CC       debug-info.lo
    /home/simark/src/babeltrace/src/plugins/lttng-utils/debug-info/debug-info.c: In function ‘debug_info_msg_iter_next’:
    /home/simark/src/babeltrace/src/plugins/lttng-utils/debug-info/debug-info.c:181:19: error: potential null pointer dereference [-Werror=null-dereference]
      181 |  bt_logging_level log_level = bin->log_level;
          |                   ^~~~~~~~~

Putting a BT_ASSERT(bin) convinces gcc that bin won't be NULL, and if it
ever happens to be NULL (because we mess up), the crash will be
smoother.

This is the last instance of the -Wnull-dereference, so we can remove
-Wno-null-dereference from configure.ac.

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

ctf: define yystrlen to strlen

Building with -Wnull-dereference on cygwin gives:

      LEX      lexer.c
      CC       libctf_parser_la-lexer.lo
      CC       libctf_parser_la-parser.lo
    parser.c: In function ‘yysyntax_error’:
    parser.c:2566:24: error: potential null pointer dereference [-Werror=null-dereference]
       for (yylen = 0; yystr[yylen]; yylen++)
                       ~~~~~^~~~~~~

For some reason, the conditional used by bison to define yystrlen makes
it so that cygwin uses the bison-provided version instead of strlen:

    # ifndef yystrlen
    #  if defined __GLIBC__ && defined _STRING_H
    #   define yystrlen strlen
    #  else
    /* Return the length of YYSTR.  */
    static YYSIZE_T
    yystrlen (const char *yystr)
    {
      YYSIZE_T yylen;
      for (yylen = 0; yystr[yylen]; yylen++)
        continue;
      return yylen;
    }
    #  endif
    # endif

Actually, probably because cygwin's string.h uses _STRING_H_ instead of
_STRING_H as its include guard.

As far as I know, strlen on cygwin (and on all the platforms we target)
is reliable.  So we can bypass this by defining yystrlen to strlen
directly.

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

ctf: Fix one -Wnull-dereference warning

When building at -O2 with gcc, I get:

      CC       visitor-generate-ir.lo
    /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/visitor-generate-ir.c: In function ‘auto_map_fields_to_trace_clock_class’:
    /home/simark/src/babeltrace/src/plugins/ctf/common/metadata/visitor-generate-ir.c:3498:22: error: potential null pointer dereference [-Werror=null-dereference]
     3498 |   if (strcmp(named_fc->name->str, field_name) == 0) {
          |              ~~~~~~~~^~~~~~

Purely based on control flow analysis, named_fc could be NULL at this
point if none of the conditional branches above are taken.  It's
logically not supposed to happen, unless in case of a programming error
on our part.

We can silence this warning and indicate that this would be an abnormal
situation by aborting if root_fc->type is neither
CTF_FIELD_CLASS_TYPE_STRUCT nor CTF_FIELD_CLASS_TYPE_VARIANT.

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

ctf, ctf-writer: Fix -Wnull-dereference warnings

When building at the -O2 optimization level with gcc, I get this:

/home/simark/src/babeltrace/src/ctf-writer/resolve.c: In function ‘resolve_type’:
/home/simark/src/babeltrace/src/ctf-writer/resolve.c:541:7: error: potential null pointer dereference [-Werror=null-dereference]
  541 |   int cur_index = type_stack_at(ctx->type_stack,
      |       ^~~~~~~~~

gcc sees that type_stack_at can possibly return NULL, but we dereference
it unconditionally.  All callers of type_stack_at don't check NULL, so
that is a good sign that it never happens.  From what I understand, if
it were to happen it would be a programming error, not a legitimate
error.  So I think we can simplify this code by making it a precondition
of calling type_stack_at that `stack` should not be NULL and `index`
should represent a valid element of `stack`.  This gets rid of the
warning at the same time.

There is the same pattern in the ctf plugin code, so fix it there too.

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

Fix -Wjump-misses-init warnings

Fix gcc warnings of this kind:

      CC       stream-class.lo
    /home/simark/src/babeltrace/src/ctf-writer/stream-class.c: In function ‘bt_ctf_stream_class_common_add_event_class’:
    /home/simark/src/babeltrace/src/ctf-writer/stream-class.c:177:3: error: jump skips variable initialization [-Werror=jump-misses-init]
      177 |   goto end;
          |   ^~~~
    /home/simark/src/babeltrace/src/ctf-writer/stream-class.c:389:1: note: label ‘end’ defined here
      389 | end:
          | ^~~
    /home/simark/src/babeltrace/src/ctf-writer/stream-class.c:241:29: note: ‘query’ declared here
      241 |  struct bt_ctf_search_query query = { .value = event_class, .found = 0 };
          |                             ^~~~~

Fix it by moving the declarations near the top, as our coding style
prescribes.

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

Fix -Wmissing-include-dirs warnings

We get this warning:

      CC       details.lo
    cc1: error: /home/smarchi/src/babeltrace/plugins: No such file or directory [-Werror=missing-include-dirs]

And indeed, src/plugins/text/details/Makefile.am sets a -I pointing to a
directory that doesn't exist anymore, so just remove it.

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

Fix -Wduplicated-cond warnings

Fixes this:

      CC       libctf_parser_la-parser.lo
    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y: In function ‘yyparse’:
    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:1313:50: error: duplicated ‘if’ condition [-Werror=duplicated-cond]
        } else if ($$->u.unary_expression.type == UNARY_UNSIGNED_CONSTANT) {
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:1309:43: note: previously used here
        if ($$->u.unary_expression.type == UNARY_UNSIGNED_CONSTANT) {
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

I am pretty sure that this line should use UNARY_SIGNED_CONSTANT.

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

black: run `black` version 19.10b0 on entire project

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

Fix: log.h: missing defines of `_ERRNO()` macros to `_UNUSED()`

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

Fix -Wsuggest-attribute warnings

plan_skip_all unconditionally exits, so it can be marked as noreturn.

Fixes:

  CC       tap.lo
/home/smarchi/src/babeltrace/tests/utils/tap/tap.c: In function ‘plan_skip_all’:
/home/smarchi/src/babeltrace/tests/utils/tap/tap.c:231:1: error: function might be candidate for attribute ‘noreturn’ [-Werror=suggest-attribute=noreturn]
 plan_skip_all(const char *reason)
 ^~~~~~~~~~~~~

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

Fix -Wstrict-prototypes warnings

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

configure: allow adding compiler-specific warning flags

Goal
----

The goal of this patch is to add a system where we can provide a bunch
of warning flags, and it will check if the current compiler supports
each of them individually.

This is inspired by how GDB's configure does:

  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/warning.m4;h=c9e64a1836a8e01339ca4e3c3d689657ff9ef92c;hb=5c49f2cd78c69d50bc7c7119596a226f05939d06#l19

GDB's macro is homegrown, but unfortunately we can't just copy it
because of the license (it's GPLv3).  We could write our own macro that
does the same thing, it's not terribly complicated.  But rather than
writing our own macro...

Implementation
--------------

There is a macro in the Autoconf Archive, AX_COMPILER_FLAGS, which does
pretty much that.  You can pass it a list of flags and it will only keep
the ones supported by the compiler.

However, it also provides a bunch of arbitrary warning flags of its own.
Some of them we want, some of them we probably don't want, and some of
them we want but the codebase is not ready for them yet.

To keep this patch reasonable, I chose to pass all the -Wno-* flags
needed to disable the warnings we have currently.  Over time, we can
gradually fix the warnings and remove them from that list.

The documentation for AX_COMPILER_FLAGS specifically says:

    The set of base and enabled flags can be augmented using the
    EXTRA-*-CFLAGS and EXTRA-*-LDFLAGS variables, which are tested and
    appended to the output variable if –enable-compile-warnings is not
    "no".  Flags should not be disabled using these arguments, as the
    entire point of AX_COMPILER_FLAGS is to enforce a consistent set of
    useful compiler warnings on code, using warnings which have been
    chosen for low false positive rates. If a compiler emits false
    positives for a warning, a #pragma should be used in the code to
    disable the warning locally.

However, there's no way we're going to add so many pragmas, some of them
for warnings we might not even want.  So I took the liberty to use the
EXTRA-YES-CFLAGS to pass the arguments to disable those warnings.

Notes
-----

(1)

I have fixed the few occurences where we pass a const pointer as a
non-const pointer (e.g.: error: passing argument 2 of ‘g_spawn_sync’
from incompatible pointer type), because with gcc 4.8 there is no way of
silencing that warning.

(2)

As explained in the comment in configure.ac, I have moved the glib
sizeof(size_t) check before the call to AX_COMPILER_FLAGS.  This is so
the sizeof(size_t) check is not affected by the strict warning flags
enabled by AX_COMPILER_FLAGS.

In practice, the problem is that the program generated by
AC_LANG_PROGRAM defines main as "int main()" instead of
"int main(void)", causing a -Wold-style-declaration warning.

(3)

As explained in the comment in configure.ac, I have explicitly enabled
-Wold-style-declaration.  I have fixed the offender.

(4)

This macro adds the following arguments to configure:

  --enable-compile-warnings=[no/yes/error]
                          Enable compiler warnings and errors
  --disable-Werror        Unconditionally make all compiler warnings non-fatal

(5)

The AX_COMPILER_FLAGS macro also provides some useful LDFLAGS.  However,
it provides -Wl,--no-as-needed, and we specifically use -Wl,--as-needed
when building babeltrace2.exe.  I chose not to use the LDFLAGS from
AX_COMPILER_FLAGS for now, by fear that it will break things in a subtle
way.

Alternatives
------------

If this is deemed too invasive, alternatives would be to:

- write our own macro, like the GDB one
- copy the AX_COMPILER_FLAGS and modify it to only keep what we want

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

Fix: tests: add cli/params/test_params to Makefile and fix it

The test bt_plugin_params.py has been broken for a while, because guess
what?  It's not ran by the testsuite!

So first, add it to the Makefile.

Then, update the value types expected by the component class.  I made it
so it raises an exception if it gets an object of a type it doesn't
expect.  That should make the test a bit more robust.

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

Cleanup: babeltrace2-cfg-cli-args.c: coding style

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

Cleanup: usages of bt_value_array_borrow_element_by_index{,_const}()

Checking the return value of this function against NULL is mostly
useless because in case of an out-of-bound access the function would
return a pointer value passed the end of the array (junk).

This commit removes NULL checks and `BT_ASSERT()`on the return value of
this function.

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

Fix: usage of `bt_value_array_get_length()`

Background
==========
Commit 601b0d3 changed the return value of the
`bt_value_array_get_size()` function now named
`bt_value_array_get_length()` from int64_t to uint64_t.

Problems
========
1. Multiple places in the code use the wrong storage type to store the
result of this function. Not only did we use `int64_t` but also, `int`,
`unsigned int` and `size_t` at some places.

2. A few places check for negative return value for handling potential
errors of this function. Because this function cannot fail and cannot
return a negative number, this is useless.

3. The babeltrace2.c file (`set_stream_intersections()` function)
contains two instances of checks against a negative value that would log
an error about the array being empty. Those log statements are
unreachable because the type is unsigned.

Solution
========
1. This commit cleanups so that we use `uint64_t` to store these value
and that we are using `uint64_t` for indexes looping on such value.

2. Remove useless error checks.

3. Those log statements are useful, so this commit corrects the
condition to compare against 0. But, this means that there is a change
in behaviour here. For example, before this commit, when doing the
stream intersection on an empty `bt_value_array` of traces the function would
return success and not log any error; now it fails and log an error.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I611bae82b4cb7092f13373283f358d8f7d13ae9d
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2249
Tested-by: jenkins <jenkins@lttng.org>

lib: make `bt_attributes_get_count()` return uint64_t

This function cannot return a negative number and cannot fail as it uses
the `bt_value_array_get_length()` function.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I1dfb06ffceaab9ef1c1ab6fa11041a2c3ddeb464
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2248
Tested-by: jenkins <jenkins@lttng.org>

Use typeof instead of __auto_type

We need to support gcc 4.8, which doesn't support __auto_type.  We can
use typeof instead.

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

Fix: src.ctf.fs: free ds_file_info when add_ds_file_to_ds_file_group fails

When add_ds_file_to_ds_file_group fails, the created ds_file_info is not
freed, resulting in this leak:

==16942== 168 (16 direct, 152 indirect) bytes in 1 blocks are definitely lost in loss record 11 of 29
==16942==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16942==    by 0x534BB10: g_malloc0 (gmem.c:129)
==16942==    by 0x7795E39: ctf_fs_ds_file_info_create (fs.c:558)
==16942==    by 0x7795E39: add_ds_file_to_ds_file_group (fs.c:781)
==16942==    by 0x7795E39: create_ds_file_groups (fs.c:954)
==16942==    by 0x7795E39: ctf_fs_trace_create (fs.c:1100)
==16942==    by 0x7795E39: ctf_fs_component_create_ctf_fs_trace_one_path (fs.c:1183)
==16942==    by 0x7795E39: ctf_fs_component_create_ctf_fs_trace (fs.c:2059)
==16942==    by 0x7797044: ctf_fs_create (fs.c:2352)
==16942==    by 0x7797044: ctf_fs_init (fs.c:2386)
==16942==    by 0x4E7172F: add_component_with_init_method_data (graph.c:1330)
==16942==    by 0x4E76213: bt_graph_add_source_component_with_initialize_method_data (graph.c:1405)
==16942==    by 0x4E762D0: bt_graph_add_source_component (graph.c:1417)
==16942==    by 0x1149A9: cmd_run_ctx_create_components_from_config_components (babeltrace2.c:2313)
==16942==    by 0x110353: cmd_run_ctx_create_components (babeltrace2.c:2405)
==16942==    by 0x110353: cmd_run (babeltrace2.c:2519)
==16942==    by 0x110353: main (babeltrace2.c:2814)

This can happen if the index fails to build, as in:

  $ ./src/cli/babeltrace2 /home/smarchi/lttng-traces/auto-20191021-162849/ust/uid/1000/64-bit

If add_ds_file_to_ds_file_group is successful, it transfers its
ownership of the ds_file_info object to the ds_file_group, in which case
it should not free it.  But if it fails, the ds_file_info object should
be freed.

This patch introduces the BT_MOVE_REF macro, analogous to C++'s
std::move, for cases like these.  It passes the value of a pointer
variable to a function, while setting that variable to NULL.  This is
meant to be used when the caller transfers its owning reference to the
callee.

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

Fix: define macros for logging levels

Compiling with -Wundef, we get:

      CC       common.lo
    In file included from /home/smarchi/src/babeltrace/src/common/common.c:27:0:
    /home/smarchi/src/babeltrace/src/logging/log.h:58:24: warning: "BT_LOGGING_LEVEL_TRACE" is not defined, evaluates to 0 [-Wundef]
     #define BT_LOG_TRACE   BT_LOGGING_LEVEL_TRACE
                            ^
    /home/smarchi/src/babeltrace/src/logging/log.h:617:35: note: in definition of macro ‘BT_LOG_ENABLED’
     #define BT_LOG_ENABLED(lvl)     ((lvl) >= _BT_MINIMAL_LOG_LEVEL)
                                       ^~~
    /home/smarchi/src/babeltrace/src/logging/log.h:618:48: note: in expansion of macro ‘BT_LOG_TRACE’
     #define BT_LOG_ENABLED_TRACE    BT_LOG_ENABLED(BT_LOG_TRACE)
                                                    ^~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:852:5: note: in expansion of macro ‘BT_LOG_ENABLED_TRACE’
     #if BT_LOG_ENABLED_TRACE
         ^~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:59:24: warning: "BT_LOGGING_LEVEL_DEBUG" is not defined, evaluates to 0 [-Wundef]
     #define BT_LOG_DEBUG   BT_LOGGING_LEVEL_DEBUG
                            ^
    ../../src/common/config.h:26:30: note: in expansion of macro ‘BT_LOG_DEBUG’
     #define BT_MINIMAL_LOG_LEVEL BT_LOG_DEBUG
                                  ^~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:92:32: note: in expansion of macro ‘BT_MINIMAL_LOG_LEVEL’
      #define _BT_MINIMAL_LOG_LEVEL BT_MINIMAL_LOG_LEVEL
                                    ^~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:617:43: note: in expansion of macro ‘_BT_MINIMAL_LOG_LEVEL’
     #define BT_LOG_ENABLED(lvl)     ((lvl) >= _BT_MINIMAL_LOG_LEVEL)
                                               ^~~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:618:33: note: in expansion of macro ‘BT_LOG_ENABLED’
     #define BT_LOG_ENABLED_TRACE    BT_LOG_ENABLED(BT_LOG_TRACE)
                                     ^~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:852:5: note: in expansion of macro ‘BT_LOG_ENABLED_TRACE’
     #if BT_LOG_ENABLED_TRACE
         ^~~~~~~~~~~~~~~~~~~~

This shows that while BT_LOGGING_LEVEL_* are not preprocessor macros,
they are being used in preprocessor conditions.  This makes the logging
macros (e.g. BT_LOGT) always defined to the concrete logging code, even
though the minimal compile-time log level should make them defined to
nothing.

Fix that by defining macros for logging levels (not exposed in the API),
which we access in log.c, the library's logging system.  The macros are
defined in a new file, logging-defs.h, because it's not possible to
include logging.h twice.

Now that the BT_LOG_ENABLED_TRACE macro is working as intended, it then
exposes this problem when building with the default setting of
BABELTRACE_MINIMAL_LOG_LEVEL=DEBUG.

    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:52:0: error: "YYDEBUG" redefined [-Werror]
     # define YYDEBUG 0

    In file included from /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:44:0:
    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.h:37:0: note: this is the location of the previous definition
     # define YYDEBUG 1

When building parser.c, parser.h is first included and defines YYDEBUG
with:

    #ifndef YYDEBUG
    # define YYDEBUG 1
    #endif

Then, we come with:

    #if BT_LOG_ENABLED_TRACE
    # define YYDEBUG 1
    # define YYFPRINTF(_stream, _fmt, args...) BT_LOGT(_fmt, ## args)
    #else
    # define YYDEBUG 0
    #endif

If we want to control YYDEBUG based on BT_LOG_ENABLED_TRACE, we need to
define that before including parser.h.  But that's not the end of our
troubles!  If YYDEBUG is defined as 0 when building parser.c, it will
not generated a definition for the variable yydebug.  Some other files
declare their own "extern int yydebug;" unconditionally and access it.
If yydebug has no definition, this results in undefined references to
yydebug.

To solve it, I want to make all files who use yydebug include some file
to obtain the yydebug declaration, which will be properly gated with the
`#if BT_LOG_ENABLED_TRACE` condition.  That new file is `parser-wrap.h`.
To make sure that nobody includes parser.h directly (which would lead to
yydebug always being declared for them), I've made it so you need to
define a special macro (ALLOW_INCLUDE_PARSER_H) to include it, which
only parser-wrap.h does.

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

Tests: debug-info: compare output of `CompleteSrc`

This source is intended to produce all type for fields and options so to
test that the `flt.lttng-utils.debug-info` copies them accurately.

On the long term, I see this CompleteSrc containing cases of all trace
IR options of all metadata objects (e.g. stream class, trace class,
etc.)

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

Tests: debug-info: compare without `debug-info` component

I find it useful to compare the output of a graph processing a regular
CTF trace (with no debugging information) with and without a
`debug-info` component.

Since those traces don't have the necessary debugging information for
the `debug-info` component to add something, the output of such graphs
should be identical.

This confirms that the `debug-info` component copies the traces
accurately.

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