drtracing.org Git - babeltrace.git/log

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>

debug-info: have `copy_*_content()` function return _STATUS

This commit also changes the uses of error logging macros to append the
cause of the failure.

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

flt.lttng-utils.debug-info: add all SC and EC to output trace class ASAP

The fact that we are copying metadata only when encountering the data
associated to it leads to variations in the output of a
`sink.text.details` component when changing message batch size. It is
not wrong it's simply annoying.

So to improve that, this commit makes `flt.lttng-utils.debug-info`
components copy all the metadata of the input trace class as soon as it
encounters it.

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

Fix: param-validation: remove memory leaks

The GArray allocated in bt_param_validation_validate is never freed, fix
that. Furthermore, there's a spurious call to
g_ptr_array_new_with_free_func that shouldn't be there, probably the
result of a bad merge or bad edit, causing another leak. Remove that
one.

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

Fix: shadowed variables

This fixes some shadowing of variable across the project, as reported by
the gcc option `-Werror=shadow=local`.

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

src.ctf.fs: append error causes in ctf_fs_file_open

I hit an fopen failure in ctf_fs_file_open and noticed we were missing
some error causes for the message to be precise.  This patch adds them.

This is the result I now get when fopen fails:

    ...
    CAUSED BY [auto-disc-source-ctf-fs-37: 'source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/file.c:98)
      Cannot open file: Too many open files: path=/tmp/lttng-traces/000300/mon_10.196.130.21-20180202-144403/ust/uid/0/64-bit/ch-1_4, mode=rb

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

lib: add _msg parameters to _ERRNO logging macros

The macros BT_COMP_LOGE_APPEND_CAUSE_ERRNO and
BT_COMP_CLASS_LOGE_APPEND_CAUSE_ERRNO are missing the `_msg` parameter
that other _ERRNO macros have. It still works, because the following
parameters are passed through the variable arguments, but it makes the
macro unnecessarily complicated to use.

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

lib: remove plugin's ABI version

We don't need this version property.

We intend for Babeltrace 2.y to be able to load any plugin written for
Babeltrace 2.x, where y ≥ x.

With this patch, `struct __bt_plugin_descriptor` only contains the
plugin's name, because this is the only required property. Optional
properties are within an array of plugin descriptor attributes
(`struct __bt_plugin_descriptor_attribute`). Each plugin descriptor
attribute has a type (`enum __bt_plugin_descriptor_attribute_type`), so
this is how we can introduce new, optional plugin properties, instead of
relying on some ABI version.

The same goes for component class attributes
(`struct __bt_plugin_component_class_descriptor_attribute`) and their
type
(`enum __bt_plugin_component_class_descriptor_attribute_type`).

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

Remove unused `src/lib/trace-ir/clock-snapshot-set.h`

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

lib: remove unused bt_graph_remove_unconnected_component()

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

Always evaluate BT_ASSERT(); add BT_ASSERT_DBG() for debug mode only

This patch makes BT_ASSERT() always evaluate its condition.

The goal is to make a lot of _slow path_ assertions evaluated whatever
the debug or non-debug mode. This will help detect project programming
errors and hopefully end users will report them.

What used to be BT_ASSERT() is now BT_ASSERT_DBG(), that is, only
evaluated in debug mode. This is similar to how BT_ASSERT_PRE() is
always evaluated while BT_ASSERT_PRE_DEV() is only evaluated in
developer mode.

I went over each single BT_ASSERT() statement in `src` and decided
whether to use BT_ASSERT() or BT_ASSERT_DBG(). My strategy is similar
to what we do for BT_ASSERT_PRE(), that is:

* Use BT_ASSERT_DBG() in anything potentially executed once or more per
  _event_ message.

  Other messages occur so infrequently compared to event messages that
  we don't care.

* Use BT_ASSERT_DBG() in property getters and object borrowing
  functions.

  We don't know how frequently the user can call those, so we don't take
  any chance.

Everything else uses BT_ASSERT(), for example:

* In the library and plugins, everything related to metadata objects.
* Everything related to graph topology.
* Everything in the CLI.
* Network communication functions in `src.ctf.lttng-live`.

I believe some BT_ASSERT_DBG() statements could still be converted
BT_ASSERT(), but this patch is a good starting point.

I left the whole CTF writer code with BT_ASSERT_DBG() for the moment
as this library is not a priority.

All the tests use BT_ASSERT() because they are not an end user use case;
we don't care if they are less efficient in production mode.

This change does not seem to affect the production build's performance;
I compared, before and after, and I do not get a run time difference
that's greater than the observed measurement error.

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

lib: remove includes from logging.h

Nothing in logging.h uses anything from those includes.

Removing them causes this failure in py-common.c:

  CC       py-common.lo
In file included from /home/smarchi/src/babeltrace/src/py-common/py-common.c:32:0:
/home/smarchi/src/babeltrace/src/py-common/py-common.h:44:18: error: unknown type name ‘bool’; did you mean ‘_Bool’?
   int log_level, bool chain);
                  ^~~~
                  _Bool

Doing this change revealed a few files that were using bool without
including stdbool.h, add the includes there.

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

bt2: make log functions clear error indicator

In all call sites of logw_exception and loge_exception_append_cause, we
clear the error indicator afterwards (call PyErr_Clear). Make these
functions clear the error indicator themselves instead, and rename them
accordingly. This could help avoid forgetting to clear the indicator in
the future.

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

Fix: bt2: clear Python error indicator in trace and trace class destruction listeners

When an exception is raised in a trace or trace class destruction
listener, we hit this assert:

    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Babeltrace 2 library postcondition not satisfied; error is:
    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Destruction listener kept a reference to the trace class being destroyed: tc-addr=0x6080000027a0, tc-is-frozen=0, tc-stream-class-count=0, tc-assigns-auto-sc-id=1
    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Aborting...

The problem is that we don't clear the error indicator on the error code
path after running the user code.  The error indicator (the exception)
keeps a reference to the Python stack, which has a reference to the
trace or trace class Python object.  This, in turn has kept a reference
to the Babeltrace trace or trace class object.

The solution is to clear the error indicator, which releases all of
this.

Another problem is that calling loge_exception_append_cause appends an
error cause, even though it's not valid to leave an error cause when
exiting those functions, as they don't return an error status.  That
error cause is left dangling and might wrongfully appear on an eventual
future error stack.

To fix it, use logw_exception instead of loge_exception_append_cause.
This matches what we do in component_class_finalize and
component_class_message_iterator_finalize, which are very similar
situations.

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

bt2: normalize the code to use some commonly used patterns

This patch changes some functions in the Python bindings native code to
use some error handling patterns that are used elsewhere in the project.
I think they help avoid some mistakes, and always using the same
patterns helps to spot mistakes more easily.

- Don't initialize status/ret variables to OK when declaring them.  If
we do this and forget to assign them in some error path, we'll
wrongfully return OK.  By not initializing them, if we forget to set
them on some error path, the compilers or static analyzers will be able
to identify it as a problem because the variable will be used without
being initialized on that path.
- When checking for Python API calls failure, check if the returned
pointer is NULL instead of checking PyErr_Occurred().  It is equivalent,
but it's preferable to do it the same way everywhere.
- After each API call, have an error check with a "goto end;".

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

bt2: rename exception handling functions

It's currently not very clear if the Python exception handling functions
append a Babeltrace error cause or clear the Python error indicator.
It's therefore easy to mis-use them.

Rename them to better indicate what they do.

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

src.ctf.fs: error out when failing to create index

This patch makes src.ctf.fs fail if it is not able to build an index for
a data stream file.  This is generally a sign of a corrupted trace.

The plan is to be strict by default (abort if we find the trace looks
corrupted), but to later introduce a parameter to tell the component to
make those checks not fatal.

So for now, we can assume that the index will always be present, hence
the comment change in fs.h.

Without this patch, the trace provided as a test leads to this abort:

    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_idx_file@data-stream-file.c:462 [auto-disc-source-ctf-fs] Invalid LTTng trace index file; indexed size != stream file size: file-size=16699392, total-packets-size=16896000
    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_stream_file@data-stream-file.c:596 [auto-disc-source-ctf-fs] Invalid packet size reported in file: stream="/tmp/single/ch-1_17", packet-offset=12701696, packet-size-bytes=4194304, file-size=16699392
    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS add_ds_file_to_ds_file_group@fs.c:789 [auto-disc-source-ctf-fs] Failed to index CTF stream file '/tmp/single/ch-1_17'

     (╯°□°)╯︵ ┻━┻  /home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/fs.c:1572: fix_index_lttng_event_after_packet_bug(): Assertion `index` failed.
    [1]    14522 abort      ./src/cli/babeltrace2 /tmp/single

The reason is that the index fails to build (the cumulative packet sizes
is larger than the data stream file size), but we keep going anyway.
The fix_index_lttng_event_after_packet_bug function, expecting an index
to always be present, hits that assert.

I added the problematic trace to our test trace collection, and a new
test script "test_fail" to test how the src.ctf.fs component class
handles invalid traces.

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

bt2: Force usage of MapValue object on component init

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

Fix: consider index of all files for data stream groups with multiple files

The indexes associated to data stream file groups (dsfg) with multiple
files are incorrect.

The first time we process a file for a given dsfg, the group is created
using the index of that file.  Up to that point, it's fine.  When
processing the other files for that dsfg, the index is never used.  The
result is that the index for that dsfg only covers the time range of the
first file that was parsed, instead of the whole data stream.

Fix it by merging the index of the added file in
add_ds_file_to_ds_file_group, in the case where we add the file to an
existing group.

A test is added for this, in the form of a trace infos query.  The
implementation of this query uses the index data directly.

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

Use assertRegex instead of assertRegexpMatches

I get these warnings when running against Python 3.7.4:

tests/plugins/src.ctf.fs/query/test_query_trace_info.py:192: DeprecationWarning: Please use assertRegex instead.

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

Fix: src.ctf.fs: use BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE when applicable

Some functions in src.ctf.fs use BT_COMP_LOGE_APPEND_CAUSE when they
encounter an error.  However, they can be called in component class
context (as part of query handling).  A NULL self_comp can therefore be
passed to BT_COMP_LOGE_APPEND_CAUSE, which results in assertions like:

    LIB/CUR-THREAD bt_current_thread_error_append_cause_from_component@current-thread.c:132 Babeltrace 2 library precondition not satisfied; error is:
    LIB/CUR-THREAD bt_current_thread_error_append_cause_from_component@current-thread.c:132 Component is NULL:
    LIB/CUR-THREAD bt_current_thread_error_append_cause_from_component@current-thread.c:132 Aborting...

These functions should use BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE and
receive both a self_comp and a self_comp_class.  This requires adding a
self_comp_class field to ctf_fs_create, which is mutually exclusive with
the self_comp field (since we are only in one of these contexts, not
both).

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

Cleanup: Dead assignments

Scan-build reports multiple:
Value stored to 'ret' is never read

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

Cleanup: flt.lttng-utils.debug-info: Dead assignment

Since the `event_get_payload_build_id_length()` and
`event_get_payload_build_id_value()` functions, can't fail, make them
return void.

Scan-build report:
Value stored to 'ret' is never read

Reported-by: Scan-Build - Dead assignment
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I950d0d327c19578d9b715e67417afd05f2ac2645
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2201
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

Update version to v2.0.0-rc1

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

Fix: avoid double-free in build_index_from_idx_file

If the validation at the end of build_index_from_idx_file fails, the
index_entry variable will still point to the last processed index entry.
That same entry will also have been added to the index->entries array.

In the error path, we free index_entry and the index object, which frees
that index entry twice.

Fix it by clearing index_entry after adding the entry to the index
object (the ownership is conceptually transferred).

I don't add a test with this patch, because the file that triggers this
bug now hits a bug further in the processing. That file will be added
in the testsuite when it will no longer make babeltrace crash.

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

Fix: ctf: query.c: Unchecked fclose() return value

Coverity report:
  CID 1382583 (#1 of 1): Unchecked return value (CHECKED_RETURN)12.
  check_return: Calling fclose without checking return value (as is done
  elsewhere 9 out of 11 times).

Reported-by: Coverity (1382583) Unchecked return value
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ic5f7d46992b823bcb0571853da4ca23dcefe3d35
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2207
CI-Build: Jonathan Rajotte Julien <jonathan.rajotte-julien@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

Bump required glib version to 2.28

While investigating a hang in the lttng-live source component when
running with glib 2.22.5, we have found that this version had a bug in
g_ptr_array_remove_index_fast.  When removing the last element of the
array, the free func is not called.

It was fixed in the following commit:

https://github.com/GNOME/glib/commit/5fffa39b6ae8f8faf1036fbf07de02ffe84ef099

The first major release where this is fixed is 2.26.  We went through
the platforms we need to support, and the oldest glib version we need to
support is 2.28.6, on Solaris 11.  Therefore, fix the issue with
lttng-live by bumping the required glib version to 2.28.

Change-Id: Ib56ac221dfc5ecf45eb62b837fe0b6f54032cf31
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2210
Reviewed-by: Michael Jeanson <mjeanson@efficios.com>
Reviewed-by: Jonathan Rajotte Julien <jonathan.rajotte-julien@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

tests: test eq and ne operators of fields and values against arbitrary objects and None

The binary operator tests that test comparing fields and values against
arbitrary objects and None are skipped for the eq and ne operators.
This patch modifies the tests to test those operators too. Unlike other
operators, we don't expect them to raise an Exception.

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

tests: test_field.py: enable tests using _test_binop_lhs_value_same

Tests using _test_binop_lhs_value_same, that make sure that using a
binary operator on a field doesn't change that field's value, are
currently disabled.  The problem is this line, that tries to copy the
field:

    value_before = copy.copy(self._def)

Since we have removed everything related to copy of fields, this is
unsupported.  However, the same test but for unary operators copies the
Python value instead.  Change _test_binop_lhs_value_same to do the same.

It's not clear to me if copying the value is really necessary, but I
prefer to leave it like this at the moment.

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

babeltrace2-intro(7): refer to `babeltrace2(1)` i/o `babeltrace2-convert(1)`

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I3aafc3ca85d228ecb8c185871ab51bd5c25764f9

babeltrace2-intro(7): add more changes since Babeltrace 1

Also: categorize the changes into general, CLI, CTF, LTTng live, and
library changes.

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

Cleanup: src.ctf.lttng-live: Dead assignment

scan-build report:
Value stored to 'ret' is never read

Reported-by: Scan-Build - Dead assignment
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I9ee4163e6268cd96591723a5e4f04321fa86de2f
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2180
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

tests: remove test_ctf_writer_clock.py

Commit 0fd756a43bce605875565a14c1ed1b070fa3ad94 ("Remove everything
related to the `bt2.ctf_writer` Python module") removed the Python
bindings for the CTF writer.

A corresponding test is still there, but skipped. I think it can be
removed as well.

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

Fix: flt.lttng-utils.debug-info: Dereference after null check

Coverity report:
CID 1401218 (#1 of 1): Dereference after null check (FORWARD_NULL)
4. var_deref_op: Dereferencing null pointer bin.

Reported-by: Coverity (1401218) Dereference after null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I1821a4f2e39096cb3dca75033ab9ce25f4b63127
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2167
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
CI-Build: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

doc/man/common-footer.txt: fix Git repository and bug tracker resources

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I9a33508514fc5d3bf2c991105e6c17e3014db664

Update project's README

This patch rewrites the README file as `README.adoc`.

`README.adoc` indicates:

* What Babeltrace 2 is.
* How Babeltrace 2 is different from Babeltrace 1.
* What are the project's build-time and run-time requirements.
* How to build Babeltrace 2.
* How to build Babeltrace 2 for development.
* Where the Babeltrace community is.

The file refers to the Babeltrace website for the reader to learn more
about the project.

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

doc/man/common-footer.txt: refer to `#lttng` IRC channel only

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I4c3246024184dbc024b56c98ec1162e984894cbc

doc/man/common-footer.txt: update bug tracker resource

Jérémie Galarneau, the current Babeltrace maintainer, decided that we
officially use the LTTng bug tracker instead of the LF one.

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

lib, plugins: use bt_field_class_type_is() where suitable

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

lib: use powers of two for object type enumerators

For all object type enumerators, that is, enumerators containing
`_TYPE`, use powers of two instead of sequential values.

This makes it possible to test that a given object has one or more
given types with a single bitwise operation, for example:

    bt_field_class_type fc_type = bt_field_class_get_type(my_fc);

    if (fc_type &
            (BT_FIELD_CLASS_TYPE_STATIC_ARRAY |
             BT_FIELD_CLASS_TYPE_DYNAMIC_ARRAY_WITHOUT_LENGTH_FIELD |
             BT_FIELD_CLASS_TYPE_DYNAMIC_ARRAY_WITH_LENGTH_FIELD)) {
        /* `my_fc` is an array */
    }

To make things easier, I'm also introducing one type enumerator per
"abstract" field class type. For example:

* `BT_FIELD_CLASS_TYPE_SIGNED_INTEGER`
* `BT_FIELD_CLASS_TYPE_SIGNED_ENUMERATION`
* `BT_FIELD_CLASS_TYPE_REAL`
* `BT_FIELD_CLASS_TYPE_ARRAY`
* `BT_FIELD_CLASS_TYPE_OPTION_WITH_SELECTOR_FIELD`

No field class object has those exact types, but their type can have
those bits set. For example, all the integer field class types have
bit 2 set. Then, if you need to know if a given type is conceptually
a given type, use bt_field_class_type_is(), which
returns:

    (type & type_to_check) == type_to_check

Each `BT_FIELD_CLASS_TYPE_*` enumerator conceptually matches, more or
less, a field class `bt2` Python class. For example:

* `BT_FIELD_CLASS_TYPE_ARRAY` → `bt2._ArrayFieldClass`
* `BT_FIELD_CLASS_TYPE_REAL` → `bt2._RealFieldClass`

The C to Python equivalent expressions are:

Object has a specific type:
    C:

        bt_field_class_get_type(fc) == BT_FIELD_CLASS_TYPE_X

    Python:

        type(fc) is bt2._XFieldClassConst

Object is an instance of a conceptual type:
    C:

        bt_field_class_type_is(bt_field_class_get_type(obj),
                               BT_FIELD_CLASS_TYPE_ABC)

    Python:

        isinstance(fc, bt2._AbcFieldClassConst)

Of course it's more natural in Python, but Python was not developed in
1972.

All `BT_FIELD_CLASS_TYPE_*` enumerators are `unsigned long long` values
and the non-API `__BT_FIELD_CLASS_TYPE_BIG_VALUE` is `1ULL << 62`
because we could have more than 32 enumerators eventually so I want to
make sure the `bt_field_class_type` type is a 64-bit integer for ABI
compatibility reasons.

Because the enumerators are not sequential anymore, we can't use them
directly as the indexes of lookup tables. This is why bt_field_create()
and bt_field_destroy() now check the FC type explicitly instead of using
lookup tables. Those functions are not called often anyway as field
objects are always created and destroyed from object pools.

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

lib: rename option/var. "selector" to "selector field"

This is to be parallel with the "length field" API of dynamic array FCs.

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

lib: have dedicated "dynamic array FC with/without length field" types

This is to match what is already done for option and variant field
classes: you can know with their type enumerator whether or not they
have a selector and which type it is.

Now, when you pass `NULL` as the length FC to
bt_field_class_array_dynamic_create(), you get a FC with the type
`BT_FIELD_CLASS_TYPE_DYNAMIC_ARRAY_WITHOUT_LENGTH_FIELD`. Otherwise
it is `BT_FIELD_CLASS_TYPE_DYNAMIC_ARRAY_WITH_LENGTH_FIELD`.

Now you can only call
bt_field_class_array_dynamic_with_length_field_borrow_length_field_path_const()
with a FC having the type
`BT_FIELD_CLASS_TYPE_DYNAMIC_ARRAY_WITH_LENGTH_FIELD`. The way to know
whether or not the dynamic array FC has a length field path is to check
its type.

In the Python bindings, there are dedicated
`_DynamicArrayWithLengthFieldFieldClass` and
`_DynamicArrayWithLengthFieldFieldClassConst` classes to match the
library changes. The `length_field_path` property is only available in
those.

A `sink.text.details` component now writes

Dynamic array (no length field)

and

Dynamic array (with length field)

depending on the dynamic array FC's type.

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

Fix: destroy_pretty_data: move NULL check earlier

Move the NULL check for pretty before we dereference it.

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

tests: make test_intersection use bt_cli, test error cases

The way test_intersection is written doesn't make it check whether the
babeltrace2 binary exits with success or failure.  In two cases
(nointersect and nostream), the babeltrace2 binary errors out, but it
isn't explicitly validated by the test, so it's unclear whether this is
the behavior we want.  The test just validates that nothing is output on
stdout, but that could happen because the process prints nothing and
exits with success, or because it prints an error on stderr and exits
with a failure status code.

This patch improves the test by making it use bt_cli instead of running
the babeltrace2 binary directly.  This helps when troubleshooting the
test, since bt_cli prints the full command line with which it invokes
babeltrace2.

Then, it makes the test check the exit status of babeltrace2.  In the
cases nointersect and nostream, we validate that it exits with a
non-zero status code, and that it prints an expected error string on
stderr.  It adds a relatively trivial call to
BT_COMP_CLASS_LOGE_APPEND_CAUSE in the src.ctf.fs query code to make the
error message clearer in the nostream case.

I don't know if the current behavior of babeltrace2 in these two cases
is the one we want, but the goal of this patch is merely to update the
test to better test the current behavior.  If we'd like a different
behavior, we can always modify babeltrace2 and the test later.

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

Fix: check for NULL in destroy_pretty_data

In pretty_init, if create_pretty fails, we go to the `error` label and
call destroy_pretty_data.  However, destroy_pretty_data does not accept
a NULL pointer.

My understanding is that our "destroy" functions generally check for a
NULL pointer, and return early in that case.

Change destroy_pretty_data to do just that.

Fixes Coverity:

    *** CID 1405908:  Null pointer dereferences  (FORWARD_NULL)
    /src/plugins/text/pretty/pretty.c: 588 in pretty_init()
    582      bt_self_component_set_data(self_comp, pretty);
    583
    584      status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_OK;
    585      goto end;
    586
    587     error:
    >>>     CID 1405908:  Null pointer dereferences  (FORWARD_NULL)
    >>>     Passing null pointer "pretty" to "destroy_pretty_data", which dereferences it.
    588      destroy_pretty_data(pretty);
    589
    590     end:
    591      return status;

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

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

This commit fixes a few annoying coding style violations. Mainly number
of tabs when breaking line.

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

Cleanup: flt.lttng-utils.debug-info: coding style fixes

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

Fix: avoid -Wmaybe-uninitialized warning in validate_map_value_entry

We get this warning when building with gcc at -Og:

    /home/smarchi/src/babeltrace/src/plugins/common/param-validation/param-validation.c: In function ‘validate_map_value_entry’:
    /home/smarchi/src/babeltrace/src/plugins/common/param-validation/param-validation.c:182:18: error: ‘candidate’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
       data->status = validate_value(value, &candidate->value_descr,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        data->ctx);
        ~~~~~~~~~~

Even though the code looks safe, we can modify it a little bit to avoid
that warning, and maybe even be a bit more readable.

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

Fix: actually run the param validation test

And fix the number of expected tests.

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